changelog: add this PR

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

gpt4all-chat/CHANGELOG.md

@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - Fix the local server rejecting min\_p/top\_p less than 1 ([#2996](https://github.com/nomic-ai/gpt4all/pull/2996))
 - 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))
+- Fix scroll reset upon download, model removal sometimes not working, and models.json cache location ([#3034](https://github.com/nomic-ai/gpt4all/pull/3034))
 
 ## [3.3.1] - 2024-09-27 ([v3.3.y](https://github.com/nomic-ai/gpt4all/tree/v3.3.y))
 

gpt4all-chat/qml/AddModelView.qml

@@ -441,9 +441,7 @@ Rectangle {
                                         Layout.alignment: Qt.AlignTop | Qt.AlignHCenter
                                         visible: !isDownloading && (installed || isIncomplete)
                                         Accessible.description: qsTr("Remove model from filesystem")
-                                        onClicked: {
-                                            Download.removeModel(filename);
-                                        }
+                                        onClicked: Download.removeModel(id)
                                     }
 
                                     MySettingsButton {

gpt4all-chat/qml/ModelSettings.qml

@@ -92,7 +92,7 @@ MySettingsTab {
                 enabled: root.currentModelInfo.isClone
                 text: qsTr("Remove")
                 onClicked: {
-                    ModelList.removeClone(root.currentModelInfo);
+                    ModelList.uninstall(root.currentModelInfo);
                     comboBox.currentIndex = 0;
                 }
             }

gpt4all-chat/qml/ModelsView.qml

@@ -221,9 +221,7 @@ Rectangle {
                                         Layout.alignment: Qt.AlignTop | Qt.AlignHCenter
                                         visible: !isDownloading && (installed || isIncomplete)
                                         Accessible.description: qsTr("Remove model from filesystem")
-                                        onClicked: {
-                                            Download.removeModel(filename);
-                                        }
+                                        onClicked: Download.removeModel(id)
                                     }
 
                                     MySettingsButton {

gpt4all-chat/src/database.cpp

@@ -1659,7 +1659,7 @@ void Database::scanQueue()
         if (info.isPdf()) {
             QPdfDocument doc;
             if (doc.load(document_path) != QPdfDocument::Error::None) {
-                qWarning() << "ERROR: Could not load pdf" << document_id << document_path;;
+                qWarning() << "ERROR: Could not load pdf" << document_id << document_path;
                 return updateFolderToIndex(folder_id, countForFolder);
             }
             title    = doc.metaData(QPdfDocument::MetaDataField::Title).toString();

gpt4all-chat/src/download.cpp

@@ -328,38 +328,30 @@ void Download::installCompatibleModel(const QString &modelName, const QString &a
     ModelList::globalInstance()->updateDataByFilename(modelFile, {{ ModelList::InstalledRole, true }});
 }
 
-void Download::removeModel(const QString &modelFile)
+// FIXME(jared): With the current implementation, it is not possible to remove a duplicate
+//               model file (same filename, different subdirectory) from within GPT4All
+//               without restarting it.
+void Download::removeModel(const QString &id)
 {
-    const QString filePath = MySettings::globalInstance()->modelPath() + modelFile;
-    QFile incompleteFile(ModelList::globalInstance()->incompleteDownloadPath(modelFile));
-    if (incompleteFile.exists()) {
-        incompleteFile.remove();
-    }
+    auto *modelList = ModelList::globalInstance();
 
-    bool shouldRemoveInstalled = false;
-    QFile file(filePath);
-    if (file.exists()) {
-        const ModelInfo info = ModelList::globalInstance()->modelInfoByFilename(modelFile);
-        MySettings::globalInstance()->eraseModel(info);
-        shouldRemoveInstalled = info.installed && !info.isClone() && (info.isDiscovered() || info.isCompatibleApi || info.description() == "" /*indicates sideloaded*/);
-        if (shouldRemoveInstalled)
-            ModelList::globalInstance()->removeInstalled(info);
-        Network::globalInstance()->trackEvent("remove_model", { {"model", modelFile} });
-        file.remove();
-        emit toastMessage(tr("Model \"%1\" is removed.").arg(info.name()));
-    }
+    auto info = modelList->modelInfo(id);
+    if (info.id().isEmpty())
+        return;
 
-    if (!shouldRemoveInstalled) {
-        QVector<QPair<int, QVariant>> data {
-            { ModelList::InstalledRole, false },
-            { ModelList::BytesReceivedRole, 0 },
-            { ModelList::BytesTotalRole, 0 },
-            { ModelList::TimestampRole, 0 },
-            { ModelList::SpeedRole, QString() },
-            { ModelList::DownloadErrorRole, QString() },
-        };
-        ModelList::globalInstance()->updateDataByFilename(modelFile, data);
-    }
+    Network::globalInstance()->trackEvent("remove_model", { {"model", info.filename()} });
+
+    // remove incomplete download
+    QFile(modelList->incompleteDownloadPath(info.filename())).remove();
+
+    // remove file, if this is a real model
+    if (!info.isClone())
+        QFile(info.path()).remove();
+
+    // remove model list entry
+    modelList->uninstall(info);
+
+    emit toastMessage(tr("Model \"%1\" is removed.").arg(info.name()));
 }
 
 void Download::handleSslErrors(QNetworkReply *reply, const QList<QSslError> &errors)

gpt4all-chat/src/download.h

@@ -65,7 +65,7 @@ public:
     Q_INVOKABLE void cancelDownload(const QString &modelFile);
     Q_INVOKABLE void installModel(const QString &modelFile, const QString &apiKey);
     Q_INVOKABLE void installCompatibleModel(const QString &modelName, const QString &apiKey, const QString &baseUrl);
-    Q_INVOKABLE void removeModel(const QString &modelFile);
+    Q_INVOKABLE void removeModel(const QString &id);
     Q_INVOKABLE bool isFirstStart(bool writeVersion = false) const;
 
 public Q_SLOTS:

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

@@ -21,12 +21,14 @@
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonValue>
+#include <QMutexLocker>
 #include <QNetworkRequest>
 #include <QObject>
 #include <QRegularExpression>
 #include <QSettings>
 #include <QSslConfiguration>
 #include <QSslSocket>
+#include <QStandardPaths>
 #include <QStringList>
 #include <QTextStream>
 #include <QTimer>
@@ -43,6 +45,8 @@ using namespace Qt::Literals::StringLiterals;
 
 //#define USE_LOCAL_MODELSJSON
 
+#define MODELS_JSON_VERSION "3"
+
 static const QStringList FILENAME_BLACKLIST { u"gpt4all-nomic-embed-text-v1.rmodel"_s };
 
 QString ModelInfo::id() const
@@ -257,7 +261,7 @@ int ModelInfo::maxContextLength() const
 {
     if (!installed || isOnline) return -1;
     if (m_maxContextLength != -1) return m_maxContextLength;
-    auto path = (dirpath + filename()).toStdString();
+    auto path = this->path().toStdString();
     int n_ctx = LLModel::Implementation::maxContextLength(path);
     if (n_ctx < 0) {
         n_ctx = 4096; // fallback value
@@ -281,7 +285,7 @@ int ModelInfo::maxGpuLayers() const
 {
     if (!installed || isOnline) return -1;
     if (m_maxGpuLayers != -1) return m_maxGpuLayers;
-    auto path = (dirpath + filename()).toStdString();
+    auto path = this->path().toStdString();
     int layers = LLModel::Implementation::layerCount(path);
     if (layers < 0) {
         layers = 100; // fallback value
@@ -398,7 +402,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 +426,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,
@@ -502,7 +504,7 @@ ModelList::ModelList()
     connect(MySettings::globalInstance(), &MySettings::contextLengthChanged, this, &ModelList::updateDataForSettings);
     connect(MySettings::globalInstance(), &MySettings::gpuLayersChanged, this, &ModelList::updateDataForSettings);
     connect(MySettings::globalInstance(), &MySettings::repeatPenaltyChanged, this, &ModelList::updateDataForSettings);
-    connect(MySettings::globalInstance(), &MySettings::repeatPenaltyTokensChanged, this, &ModelList::updateDataForSettings);;
+    connect(MySettings::globalInstance(), &MySettings::repeatPenaltyTokensChanged, this, &ModelList::updateDataForSettings);
     connect(MySettings::globalInstance(), &MySettings::promptTemplateChanged, this, &ModelList::updateDataForSettings);
     connect(MySettings::globalInstance(), &MySettings::systemPromptChanged, this, &ModelList::updateDataForSettings);
     connect(&m_networkManager, &QNetworkAccessManager::sslErrors, this, &ModelList::handleSslErrors);
@@ -542,7 +544,7 @@ const QList<ModelInfo> ModelList::selectableModelList() const
     // FIXME: This needs to be kept in sync with m_selectableModels so should probably be merged
     QMutexLocker locker(&m_mutex);
     QList<ModelInfo> infos;
-    for (ModelInfo *info : m_models)
+    for (auto *info : std::as_const(m_models))
         if (info->installed && !info->isEmbeddingModel)
             infos.append(*info);
     return infos;
@@ -560,7 +562,7 @@ ModelInfo ModelList::defaultModelInfo() const
     const bool hasUserDefaultName = !userDefaultModelName.isEmpty() && userDefaultModelName != "Application default";
 
     ModelInfo *defaultModel = nullptr;
-    for (ModelInfo *info : m_models) {
+    for (auto *info : std::as_const(m_models)) {
         if (!info->installed)
             continue;
         defaultModel = info;
@@ -589,7 +591,7 @@ bool ModelList::contains(const QString &id) const
 bool ModelList::containsByFilename(const QString &filename) const
 {
     QMutexLocker locker(&m_mutex);
-    for (ModelInfo *info : m_models)
+    for (auto *info : std::as_const(m_models))
         if (info->filename() == filename)
             return true;
     return false;
@@ -633,6 +635,7 @@ bool ModelList::lessThan(const ModelInfo* a, const ModelInfo* b, DiscoverSort s,
 
 void ModelList::addModel(const QString &id)
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     const bool hasModel = contains(id);
     Q_ASSERT(!hasModel);
     if (hasModel) {
@@ -804,7 +807,7 @@ QVariant ModelList::data(const QString &id, int role) const
 QVariant ModelList::dataByFilename(const QString &filename, int role) const
 {
     QMutexLocker locker(&m_mutex);
-    for (ModelInfo *info : m_models)
+    for (auto *info : std::as_const(m_models))
         if (info->filename() == filename)
             return dataInternal(info, role);
     return QVariant();
@@ -821,207 +824,209 @@ QVariant ModelList::data(const QModelIndex &index, int role) const
 
 void ModelList::updateData(const QString &id, const QVector<QPair<int, QVariant>> &data)
 {
-    int index;
-    {
-        QMutexLocker locker(&m_mutex);
-        if (!m_modelMap.contains(id)) {
-            qWarning() << "ERROR: cannot update as model map does not contain" << id;
-            return;
-        }
+    QMutexLocker lock(&m_mutex);
+    updateDataInternal(id, data, lock, /*relock*/ false);
+}
 
-        ModelInfo *info = m_modelMap.value(id);
-        index = m_models.indexOf(info);
-        if (index == -1) {
-            qWarning() << "ERROR: cannot update as model list does not contain" << id;
-            return;
-        }
+void ModelList::updateDataInternal(const QString &id, const QVector<QPair<int, QVariant>> &data,
+                                   QMutexLocker<QMutex> &lock, bool relock)
+{
+    Q_ASSERT(QThread::currentThread() == thread());
+    Q_ASSERT(lock.isLocked());
 
-        // 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;
+    if (!m_modelMap.contains(id)) {
+        qWarning() << "ERROR: cannot update as model map does not contain" << id;
+        return;
+    }
 
-        for (const auto &d : data) {
-            const int role = d.first;
-            const QVariant value = d.second;
-            switch (role) {
-            case IdRole:
-                {
-                    if (info->id() != value.toString()) {
-                        info->setId(value.toString());
-                        shouldSort = true;
-                    }
-                    break;
+    ModelInfo *info = m_modelMap.value(id);
+    int index = m_models.indexOf(info);
+    if (index == -1) {
+        qWarning() << "ERROR: cannot update as model list does not contain" << id;
+        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;
+        switch (role) {
+        case IdRole:
+            {
+                if (info->id() != value.toString()) {
+                    info->setId(value.toString());
+                    shouldSort = true;
                 }
-            case NameRole:
-                info->setName(value.toString()); break;
-            case FilenameRole:
-                info->setFilename(value.toString()); break;
-            case DirpathRole:
-                info->dirpath = value.toString(); break;
-            case FilesizeRole:
-                info->filesize = value.toString(); break;
-            case HashRole:
-                info->hash = value.toByteArray(); break;
-            case HashAlgorithmRole:
-                info->hashAlgorithm = static_cast<ModelInfo::HashAlgorithm>(value.toInt()); break;
-            case CalcHashRole:
-                info->calcHash = value.toBool(); break;
-            case InstalledRole:
-                info->installed = value.toBool(); break;
-            case DefaultRole:
-                info->isDefault = value.toBool(); break;
-            case OnlineRole:
-                info->isOnline = value.toBool(); break;
-            case CompatibleApiRole:
-                info->isCompatibleApi = value.toBool(); break;
-            case DescriptionRole:
-                info->setDescription(value.toString()); break;
-            case RequiresVersionRole:
-                info->requiresVersion = value.toString(); break;
-            case VersionRemovedRole:
-                info->versionRemoved = value.toString(); break;
-            case UrlRole:
-                info->setUrl(value.toString()); break;
-            case BytesReceivedRole:
-                info->bytesReceived = value.toLongLong(); break;
-            case BytesTotalRole:
-                info->bytesTotal = value.toLongLong(); break;
-            case TimestampRole:
-                info->timestamp = value.toLongLong(); break;
-            case SpeedRole:
-                info->speed = value.toString(); break;
-            case DownloadingRole:
-                info->isDownloading = value.toBool(); break;
-            case IncompleteRole:
-                info->isIncomplete = value.toBool(); break;
-            case DownloadErrorRole:
-                info->downloadError = value.toString(); break;
-            case OrderRole:
-                {
-                    if (info->order != value.toString()) {
-                        info->order = value.toString();
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case NameRole:
+            info->setName(value.toString()); break;
+        case FilenameRole:
+            info->setFilename(value.toString()); break;
+        case DirpathRole:
+            info->dirpath = value.toString(); break;
+        case FilesizeRole:
+            info->filesize = value.toString(); break;
+        case HashRole:
+            info->hash = value.toByteArray(); break;
+        case HashAlgorithmRole:
+            info->hashAlgorithm = static_cast<ModelInfo::HashAlgorithm>(value.toInt()); break;
+        case CalcHashRole:
+            info->calcHash = value.toBool(); break;
+        case InstalledRole:
+            info->installed = value.toBool(); break;
+        case DefaultRole:
+            info->isDefault = value.toBool(); break;
+        case OnlineRole:
+            info->isOnline = value.toBool(); break;
+        case CompatibleApiRole:
+            info->isCompatibleApi = value.toBool(); break;
+        case DescriptionRole:
+            info->setDescription(value.toString()); break;
+        case RequiresVersionRole:
+            info->requiresVersion = value.toString(); break;
+        case VersionRemovedRole:
+            info->versionRemoved = value.toString(); break;
+        case UrlRole:
+            info->setUrl(value.toString()); break;
+        case BytesReceivedRole:
+            info->bytesReceived = value.toLongLong(); break;
+        case BytesTotalRole:
+            info->bytesTotal = value.toLongLong(); break;
+        case TimestampRole:
+            info->timestamp = value.toLongLong(); break;
+        case SpeedRole:
+            info->speed = value.toString(); break;
+        case DownloadingRole:
+            info->isDownloading = value.toBool(); break;
+        case IncompleteRole:
+            info->isIncomplete = value.toBool(); break;
+        case DownloadErrorRole:
+            info->downloadError = value.toString(); break;
+        case OrderRole:
+            {
+                if (info->order != value.toString()) {
+                    info->order = value.toString();
+                    shouldSort = true;
                 }
-            case RamrequiredRole:
-                info->ramrequired = value.toInt(); break;
-            case ParametersRole:
-                info->parameters = value.toString(); break;
-            case QuantRole:
-                info->setQuant(value.toString()); break;
-            case TypeRole:
-                info->setType(value.toString()); break;
-            case IsCloneRole:
-                {
-                    if (info->isClone() != value.toBool()) {
-                        info->setIsClone(value.toBool());
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case RamrequiredRole:
+            info->ramrequired = value.toInt(); break;
+        case ParametersRole:
+            info->parameters = value.toString(); break;
+        case QuantRole:
+            info->setQuant(value.toString()); break;
+        case TypeRole:
+            info->setType(value.toString()); break;
+        case IsCloneRole:
+            {
+                if (info->isClone() != value.toBool()) {
+                    info->setIsClone(value.toBool());
+                    shouldSort = true;
                 }
-            case IsDiscoveredRole:
-                {
-                    if (info->isDiscovered() != value.toBool()) {
-                        info->setIsDiscovered(value.toBool());
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case IsDiscoveredRole:
+            {
+                if (info->isDiscovered() != value.toBool()) {
+                    info->setIsDiscovered(value.toBool());
+                    shouldSort = true;
                 }
-            case IsEmbeddingModelRole:
-                info->isEmbeddingModel = value.toBool(); break;
-            case TemperatureRole:
-                info->setTemperature(value.toDouble()); break;
-            case TopPRole:
-                info->setTopP(value.toDouble()); break;
-            case MinPRole:
-                info->setMinP(value.toDouble()); break;
-            case TopKRole:
-                info->setTopK(value.toInt()); break;
-            case MaxLengthRole:
-                info->setMaxLength(value.toInt()); break;
-            case PromptBatchSizeRole:
-                info->setPromptBatchSize(value.toInt()); break;
-            case ContextLengthRole:
-                info->setContextLength(value.toInt()); break;
-            case GpuLayersRole:
-                info->setGpuLayers(value.toInt()); break;
-            case RepeatPenaltyRole:
-                info->setRepeatPenalty(value.toDouble()); break;
-            case RepeatPenaltyTokensRole:
-                info->setRepeatPenaltyTokens(value.toInt()); break;
-            case PromptTemplateRole:
-                info->setPromptTemplate(value.toString()); break;
-            case SystemPromptRole:
-                info->setSystemPrompt(value.toString()); break;
-            case ChatNamePromptRole:
-                info->setChatNamePrompt(value.toString()); break;
-            case SuggestedFollowUpPromptRole:
-                info->setSuggestedFollowUpPrompt(value.toString()); break;
-            case LikesRole:
-                {
-                    if (info->likes() != value.toInt()) {
-                        info->setLikes(value.toInt());
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case IsEmbeddingModelRole:
+            info->isEmbeddingModel = value.toBool(); break;
+        case TemperatureRole:
+            info->setTemperature(value.toDouble()); break;
+        case TopPRole:
+            info->setTopP(value.toDouble()); break;
+        case MinPRole:
+            info->setMinP(value.toDouble()); break;
+        case TopKRole:
+            info->setTopK(value.toInt()); break;
+        case MaxLengthRole:
+            info->setMaxLength(value.toInt()); break;
+        case PromptBatchSizeRole:
+            info->setPromptBatchSize(value.toInt()); break;
+        case ContextLengthRole:
+            info->setContextLength(value.toInt()); break;
+        case GpuLayersRole:
+            info->setGpuLayers(value.toInt()); break;
+        case RepeatPenaltyRole:
+            info->setRepeatPenalty(value.toDouble()); break;
+        case RepeatPenaltyTokensRole:
+            info->setRepeatPenaltyTokens(value.toInt()); break;
+        case PromptTemplateRole:
+            info->setPromptTemplate(value.toString()); break;
+        case SystemPromptRole:
+            info->setSystemPrompt(value.toString()); break;
+        case ChatNamePromptRole:
+            info->setChatNamePrompt(value.toString()); break;
+        case SuggestedFollowUpPromptRole:
+            info->setSuggestedFollowUpPrompt(value.toString()); break;
+        case LikesRole:
+            {
+                if (info->likes() != value.toInt()) {
+                    info->setLikes(value.toInt());
+                    shouldSort = true;
                 }
-            case DownloadsRole:
-                {
-                    if (info->downloads() != value.toInt()) {
-                        info->setDownloads(value.toInt());
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case DownloadsRole:
+            {
+                if (info->downloads() != value.toInt()) {
+                    info->setDownloads(value.toInt());
+                    shouldSort = true;
                 }
-            case RecencyRole:
-                {
-                    if (info->recency() != value.toDateTime()) {
-                        info->setRecency(value.toDateTime());
-                        shouldSort = true;
-                    }
-                    break;
+                break;
+            }
+        case RecencyRole:
+            {
+                if (info->recency() != value.toDateTime()) {
+                    info->setRecency(value.toDateTime());
+                    shouldSort = true;
                 }
+                break;
             }
         }
-
-        // Extra guarantee that these always remains in sync with filesystem
-        QString modelPath = info->dirpath + info->filename();
-        const QFileInfo fileInfo(modelPath);
-        info->installed = fileInfo.exists();
-        const QFileInfo incompleteInfo(incompleteDownloadPath(info->filename()));
-        info->isIncomplete = incompleteInfo.exists();
-
-        // check installed, discovered/sideloaded models only (including clones)
-        if (!info->checkedEmbeddingModel && !info->isEmbeddingModel && info->installed
-            && (info->isDiscovered() || info->description().isEmpty()))
-        {
-            // read GGUF and decide based on model architecture
-            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
-    m_selectableModels->invalidate();
-    m_installedModels->invalidate();
-    m_downloadableModels->invalidate();
+    // Extra guarantee that these always remains in sync with filesystem
+    QString modelPath = info->path();
+    const QFileInfo fileInfo(modelPath);
+    info->installed = fileInfo.exists();
+    const QFileInfo incompleteInfo(incompleteDownloadPath(info->filename()));
+    info->isIncomplete = incompleteInfo.exists();
+
+    // check installed, discovered/sideloaded models only (including clones)
+    if (!info->checkedEmbeddingModel && !info->isEmbeddingModel && info->installed
+        && (info->isDiscovered() || info->description().isEmpty()))
+    {
+        // read GGUF and decide based on model architecture
+        info->isEmbeddingModel = LLModel::Implementation::isEmbeddingModel(modelPath.toStdString());
+        info->checkedEmbeddingModel = true;
+    }
+
+    lock.unlock();
+
+    emit dataChanged(createIndex(index, 0), createIndex(index, 0));
+    if (shouldSort)
+        resortModel();
 
     emit selectableModelListChanged();
+
+    if (relock)
+        lock.relock();
 }
 
 void ModelList::resortModel()
 {
-    emit layoutAboutToBeChanged();
+    const QList<QPersistentModelIndex> parents { QModelIndex() };
+    emit layoutAboutToBeChanged(parents, QAbstractItemModel::VerticalSortHint);
     {
         QMutexLocker locker(&m_mutex);
         auto s = m_discoverSort;
@@ -1030,29 +1035,30 @@ void ModelList::resortModel()
             return ModelList::lessThan(lhs, rhs, s, d);
         });
     }
-    emit layoutChanged();
+    emit layoutChanged(parents, QAbstractItemModel::VerticalSortHint);
 }
 
 void ModelList::updateDataByFilename(const QString &filename, QVector<QPair<int, QVariant>> data)
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     if (data.isEmpty())
         return; // no-op
 
-    QVector<QString> modelsById;
     {
         QMutexLocker locker(&m_mutex);
-        for (ModelInfo *info : m_models)
+        QStringList modelsById;
+        for (auto *info : std::as_const(m_models))
             if (info->filename() == filename)
                 modelsById.append(info->id());
-    }
 
-    if (modelsById.isEmpty()) {
-        qWarning() << "ERROR: cannot update model as list does not contain file" << filename;
-        return;
-    }
+        if (modelsById.isEmpty()) {
+            qWarning() << "ERROR: cannot update model as list does not contain file" << filename;
+            return;
+        }
 
-    for (const QString &id : modelsById)
-        updateData(id, data);
+        for (auto &id : std::as_const(modelsById))
+            updateDataInternal(id, data, locker, /*relock*/ &id != &modelsById.constLast());
+    }
 }
 
 ModelInfo ModelList::modelInfo(const QString &id) const
@@ -1066,7 +1072,7 @@ ModelInfo ModelList::modelInfo(const QString &id) const
 ModelInfo ModelList::modelInfoByFilename(const QString &filename) const
 {
     QMutexLocker locker(&m_mutex);
-    for (ModelInfo *info : m_models)
+    for (auto *info : std::as_const(m_models))
         if (info->filename() == filename)
             return *info;
     return ModelInfo();
@@ -1075,15 +1081,15 @@ ModelInfo ModelList::modelInfoByFilename(const QString &filename) const
 bool ModelList::isUniqueName(const QString &name) const
 {
     QMutexLocker locker(&m_mutex);
-    for (const ModelInfo *info : m_models) {
-        if(info->name() == name)
+    for (auto *info : std::as_const(m_models))
+        if (info->name() == name)
             return false;
-    }
     return true;
 }
 
 QString ModelList::clone(const ModelInfo &model)
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     const QString id = Network::globalInstance()->generateUniqueId();
     addModel(id);
 
@@ -1115,50 +1121,84 @@ QString ModelList::clone(const ModelInfo &model)
     return id;
 }
 
-void ModelList::removeClone(const ModelInfo &model)
+void ModelList::uninstall(const ModelInfo &model)
 {
-    Q_ASSERT(model.isClone());
-    if (!model.isClone())
-        return;
+    Q_ASSERT(QThread::currentThread() == thread());
 
-    removeInternal(model);
-    emit layoutChanged();
+    {
+        QMutexLocker lock(&m_mutex);
+
+        if (!model.isClone()) {
+            QStringList modelsById;
+            auto filename = model.filename();
+            for (const auto *info : std::as_const(m_models))
+                if (info->filename() == filename && info->isClone())
+                    modelsById << info->id();
+
+            for (const auto &id : std::as_const(modelsById))
+                removeInternal(id, lock);
+        }
+
+        auto id = model.id();
+        if (model.isClone() || model.isDiscovered() || model.isCompatibleApi
+            || model.description() == "" /*sideloaded*/
+        ) {
+            // model can be completely removed from list
+            removeInternal(id, lock, /*relock*/ false);
+            emit selectableModelListChanged();
+        } else {
+            // Model came from models.json and needs to be reset instead of removed
+            QVector<QPair<int, QVariant>> data {
+                { ModelList::InstalledRole,     false     },
+                { ModelList::BytesReceivedRole, 0         },
+                { ModelList::BytesTotalRole,    0         },
+                { ModelList::TimestampRole,     0         },
+                { ModelList::SpeedRole,         QString() },
+                { ModelList::DownloadErrorRole, QString() },
+            };
+            updateDataInternal(id, data, lock, /*relock*/ false);
+
+            // erase settings
+            MySettings::globalInstance()->eraseModel(id);
+        }
+    }
 }
 
-void ModelList::removeInstalled(const ModelInfo &model)
+void ModelList::removeInternal(const QString &id, QMutexLocker<QMutex> &lock, bool relock)
 {
-    Q_ASSERT(model.installed);
-    Q_ASSERT(!model.isClone());
-    Q_ASSERT(model.isDiscovered() || model.isCompatibleApi || model.description() == "" /*indicates sideloaded*/);
-    removeInternal(model);
-    emit layoutChanged();
-}
+    Q_ASSERT(QThread::currentThread() == thread());
+    Q_ASSERT(lock.isLocked());
 
-void ModelList::removeInternal(const ModelInfo &model)
-{
-    const bool hasModel = contains(model.id());
+    bool hasModel = m_modelMap.contains(id);
     Q_ASSERT(hasModel);
     if (!hasModel) {
-        qWarning() << "ERROR: model list does not contain" << model.id();
+        qWarning() << "ERROR: model list does not contain" << id;
         return;
     }
 
-    int indexOfModel = 0;
-    {
-        QMutexLocker locker(&m_mutex);
-        ModelInfo *info = m_modelMap.value(model.id());
-        indexOfModel = m_models.indexOf(info);
-    }
-    beginRemoveRows(QModelIndex(), indexOfModel, indexOfModel);
-    {
-        QMutexLocker locker(&m_mutex);
-        ModelInfo *info = m_models.takeAt(indexOfModel);
-        m_modelMap.remove(info->id());
-        delete info;
-    }
+    auto mapIt = std::as_const(m_modelMap).find(id);
+    qsizetype listIdx = m_models.indexOf(*mapIt);
+
+    lock.unlock();
+    beginRemoveRows({}, listIdx, listIdx);
+    lock.relock();
+
+    // remove entry
+    auto *info = *mapIt;
+    Q_ASSERT(std::as_const(m_modelMap).find(id) == mapIt);
+    Q_ASSERT(m_models.indexOf(info) == listIdx);
+    m_modelMap.erase(mapIt);
+    m_models.remove(listIdx);
+    delete info;
+
+    lock.unlock();
     endRemoveRows();
-    emit selectableModelListChanged();
-    MySettings::globalInstance()->eraseModel(model);
+
+    // erase settings
+    MySettings::globalInstance()->eraseModel(id);
+
+    if (relock)
+        lock.relock();
 }
 
 QString ModelList::uniqueModelName(const ModelInfo &model) const
@@ -1175,8 +1215,8 @@ QString ModelList::uniqueModelName(const ModelInfo &model) const
     int maxSuffixNumber = 0;
     bool baseNameExists = false;
 
-    for (const ModelInfo *info : m_models) {
-        if(info->name() == baseName)
+    for (auto *info : std::as_const(m_models)) {
+        if (info->name() == baseName)
             baseNameExists = true;
 
         QRegularExpressionMatch match = re.match(info->name());
@@ -1297,7 +1337,7 @@ void ModelList::processModelDirectory(const QString &path)
         QVector<QString> modelsById;
         {
             QMutexLocker locker(&m_mutex);
-            for (ModelInfo *info : m_models)
+            for (auto *info : std::as_const(m_models))
                 if (info->filename() == filename)
                     modelsById.append(info->id());
         }
@@ -1308,7 +1348,7 @@ void ModelList::processModelDirectory(const QString &path)
             modelsById.append(filename);
         }
 
-        for (const QString &id : modelsById) {
+        for (auto &id : std::as_const(modelsById)) {
             QVector<QPair<int, QVariant>> data {
                 { InstalledRole, true },
                 { FilenameRole, filename },
@@ -1333,6 +1373,7 @@ void ModelList::processModelDirectory(const QString &path)
 
 void ModelList::updateModelsFromDirectory()
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     const QString exePath = QCoreApplication::applicationDirPath() + QDir::separator();
     const QString localPath = MySettings::globalInstance()->modelPath();
 
@@ -1344,15 +1385,32 @@ void ModelList::updateModelsFromDirectory()
     }
 }
 
-#define MODELS_VERSION 3
+static QString modelsJsonFilename()
+{
+    return QStringLiteral("models" MODELS_JSON_VERSION ".json");
+}
+
+static std::optional<QFile> modelsJsonCacheFile()
+{
+    constexpr auto loc = QStandardPaths::CacheLocation;
+    QString modelsJsonFname = modelsJsonFilename();
+    if (auto path = QStandardPaths::locate(loc, modelsJsonFname); !path.isEmpty())
+        return std::make_optional<QFile>(path);
+    if (auto path = QStandardPaths::writableLocation(loc); !path.isEmpty())
+        return std::make_optional<QFile>(path);
+    return std::nullopt;
+}
 
 void ModelList::updateModelsFromJson()
 {
+    QString modelsJsonFname = modelsJsonFilename();
+
 #if defined(USE_LOCAL_MODELSJSON)
-    QUrl jsonUrl("file://" + QDir::homePath() + u"/dev/large_language_models/gpt4all/gpt4all-chat/metadata/models%1.json"_s.arg(MODELS_VERSION));
+    QUrl jsonUrl(u"file://%1/dev/large_language_models/gpt4all/gpt4all-chat/metadata/%2"_s.arg(QDir::homePath(), modelsJsonFname));
 #else
-    QUrl jsonUrl(u"http://gpt4all.io/models/models%1.json"_s.arg(MODELS_VERSION));
+    QUrl jsonUrl(u"http://gpt4all.io/models/%1"_s.arg(modelsJsonFname));
 #endif
+
     QNetworkRequest request(jsonUrl);
     QSslConfiguration conf = request.sslConfiguration();
     conf.setPeerVerifyMode(QSslSocket::VerifyNone);
@@ -1371,18 +1429,15 @@ void ModelList::updateModelsFromJson()
         qWarning() << "WARNING: Could not download models.json synchronously";
         updateModelsFromJsonAsync();
 
-        QSettings settings;
-        QFileInfo info(settings.fileName());
-        QString dirPath = info.canonicalPath();
-        const QString modelsConfig = dirPath + "/models.json";
-        QFile file(modelsConfig);
-        if (!file.open(QIODeviceBase::ReadOnly)) {
-            qWarning() << "ERROR: Couldn't read models config file: " << modelsConfig;
-        } else {
-            QByteArray jsonData = file.readAll();
-            file.close();
+        auto cacheFile = modelsJsonCacheFile();
+        if (!cacheFile) {
+            // no known location
+        } else if (cacheFile->open(QIODeviceBase::ReadOnly)) {
+            QByteArray jsonData = cacheFile->readAll();
+            cacheFile->close();
             parseModelsJsonFile(jsonData, false);
-        }
+        } else if (cacheFile->exists())
+            qWarning() << "ERROR: Couldn't read models.json cache file: " << cacheFile->fileName();
     }
     delete jsonReply;
 }
@@ -1391,12 +1446,14 @@ void ModelList::updateModelsFromJsonAsync()
 {
     m_asyncModelRequestOngoing = true;
     emit asyncModelRequestOngoingChanged();
+    QString modelsJsonFname = modelsJsonFilename();
 
 #if defined(USE_LOCAL_MODELSJSON)
-    QUrl jsonUrl("file://" + QDir::homePath() + u"/dev/large_language_models/gpt4all/gpt4all-chat/metadata/models%1.json"_s.arg(MODELS_VERSION));
+    QUrl jsonUrl(u"file://%1/dev/large_language_models/gpt4all/gpt4all-chat/metadata/%2"_s.arg(QDir::homePath(), modelsJsonFname));
 #else
-    QUrl jsonUrl(u"http://gpt4all.io/models/models%1.json"_s.arg(MODELS_VERSION));
+    QUrl jsonUrl(u"http://gpt4all.io/models/%1"_s.arg(modelsJsonFname));
 #endif
+
     QNetworkRequest request(jsonUrl);
     QSslConfiguration conf = request.sslConfiguration();
     conf.setPeerVerifyMode(QSslSocket::VerifyNone);
@@ -1459,23 +1516,20 @@ void ModelList::parseModelsJsonFile(const QByteArray &jsonData, bool save)
     }
 
     if (save) {
-        QSettings settings;
-        QFileInfo info(settings.fileName());
-        QString dirPath = info.canonicalPath();
-        const QString modelsConfig = dirPath + "/models.json";
-        QFile file(modelsConfig);
-        if (!file.open(QIODeviceBase::WriteOnly)) {
-            qWarning() << "ERROR: Couldn't write models config file: " << modelsConfig;
-        } else {
-            file.write(jsonData);
-            file.close();
-        }
+        auto cacheFile = modelsJsonCacheFile();
+        if (!cacheFile) {
+            // no known location
+        } else if (QFileInfo(*cacheFile).dir().mkpath(u"."_s) && cacheFile->open(QIODeviceBase::WriteOnly)) {
+            cacheFile->write(jsonData);
+            cacheFile->close();
+        } else
+            qWarning() << "ERROR: Couldn't write models config file: " << cacheFile->fileName();
     }
 
     QJsonArray jsonArray = document.array();
     const QString currentVersion = QCoreApplication::applicationVersion();
 
-    for (const QJsonValue &value : jsonArray) {
+    for (auto &value : std::as_const(jsonArray)) {
         QJsonObject obj = value.toObject();
 
         QString modelName = obj["name"].toString();
@@ -1735,6 +1789,7 @@ void ModelList::parseModelsJsonFile(const QByteArray &jsonData, bool save)
 
 void ModelList::updateDiscoveredInstalled(const ModelInfo &info)
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     QVector<QPair<int, QVariant>> data {
         { ModelList::InstalledRole, true },
         { ModelList::IsDiscoveredRole, true },
@@ -1755,7 +1810,7 @@ void ModelList::updateModelsFromSettings()
 {
     QSettings settings;
     QStringList groups = settings.childGroups();
-    for (const QString &g: groups) {
+    for (auto &g : std::as_const(groups)) {
         if (!g.startsWith("model-"))
             continue;
 
@@ -1919,16 +1974,14 @@ void ModelList::setDiscoverSort(DiscoverSort sort)
 void ModelList::clearDiscoveredModels()
 {
     // NOTE: This could be made much more efficient
-    QList<ModelInfo> infos;
-    {
-        QMutexLocker locker(&m_mutex);
-        for (ModelInfo *info : m_models)
-            if (info->isDiscovered() && !info->installed)
-                infos.append(*info);
-    }
-    for (ModelInfo &info : infos)
-        removeInternal(info);
-    emit layoutChanged();
+    QMutexLocker locker(&m_mutex);
+    QStringList ids;
+    for (auto *info : std::as_const(m_models))
+        if (info->isDiscovered() && !info->installed)
+            ids << info->id();
+
+    for (auto &id : std::as_const(ids))
+        removeInternal(id, locker, /*relock*/ &id != &ids.constLast());
 }
 
 float ModelList::discoverProgress() const
@@ -1945,6 +1998,7 @@ bool ModelList::discoverInProgress() const
 
 void ModelList::discoverSearch(const QString &search)
 {
+    Q_ASSERT(QThread::currentThread() == thread());
     Q_ASSERT(!m_discoverInProgress);
 
     clearDiscoveredModels();
@@ -2053,7 +2107,7 @@ void ModelList::parseDiscoveryJsonFile(const QByteArray &jsonData)
 
     QJsonArray jsonArray = document.array();
 
-    for (const QJsonValue &value : jsonArray) {
+    for (auto &value : std::as_const(jsonArray)) {
         QJsonObject obj = value.toObject();
         QJsonDocument jsonDocument(obj);
         QByteArray jsonData = jsonDocument.toJson();
@@ -2061,7 +2115,7 @@ void ModelList::parseDiscoveryJsonFile(const QByteArray &jsonData)
         QString repo_id = obj["id"].toString();
         QJsonArray siblingsArray = obj["siblings"].toArray();
         QList<QPair<QuantType, QString>> filteredAndSortedFilenames;
-        for (const QJsonValue &sibling : siblingsArray) {
+        for (auto &sibling : std::as_const(siblingsArray)) {
 
             QJsonObject s = sibling.toObject();
             QString filename = s["rfilename"].toString();
@@ -2100,7 +2154,7 @@ void ModelList::parseDiscoveryJsonFile(const QByteArray &jsonData)
     emit discoverProgressChanged();
     if (!m_discoverNumberOfResults) {
         m_discoverInProgress = false;
-        emit discoverInProgressChanged();;
+        emit discoverInProgressChanged();
     }
 }
 
@@ -2176,9 +2230,8 @@ void ModelList::handleDiscoveryItemFinished()
     emit discoverProgressChanged();
 
     if (discoverProgress() >= 1.0) {
-        emit layoutChanged();
         m_discoverInProgress = false;
-        emit discoverInProgressChanged();;
+        emit discoverInProgressChanged();
     }
 
     reply->deleteLater();

gpt4all-chat/src/modellist.h

@@ -23,12 +23,15 @@
 
 using namespace Qt::Literals::StringLiterals;
 
+template <typename> class QMutexLocker;
+
 
 struct ModelInfo {
     Q_GADGET
     Q_PROPERTY(QString id READ id WRITE setId)
     Q_PROPERTY(QString name READ name WRITE setName)
     Q_PROPERTY(QString filename READ filename WRITE setFilename)
+    Q_PROPERTY(QString path READ path)
     Q_PROPERTY(QString dirpath MEMBER dirpath)
     Q_PROPERTY(QString filesize MEMBER filesize)
     Q_PROPERTY(QByteArray hash MEMBER hash)
@@ -92,6 +95,9 @@ public:
     QString filename() const;
     void setFilename(const QString &name);
 
+    // FIXME(jared): This is the one true path of the model. Never use anything else.
+    QString path() const { return dirpath + filename(); }
+
     QString description() const;
     void setDescription(const QString &d);
 
@@ -119,6 +125,7 @@ public:
     QDateTime recency() const;
     void setRecency(const QDateTime &r);
 
+    // FIXME(jared): a class with getters should not also have public mutable fields
     QString dirpath;
     QString filesize;
     QByteArray hash;
@@ -419,8 +426,8 @@ public:
     Q_INVOKABLE ModelInfo modelInfoByFilename(const QString &filename) const;
     Q_INVOKABLE bool isUniqueName(const QString &name) const;
     Q_INVOKABLE QString clone(const ModelInfo &model);
-    Q_INVOKABLE void removeClone(const ModelInfo &model);
-    Q_INVOKABLE void removeInstalled(const ModelInfo &model);
+    // delist a model and erase its settings, or mark it as not installed if built-in
+    void uninstall(const ModelInfo &model);
     ModelInfo defaultModelInfo() const;
 
     void addModel(const QString &id);
@@ -495,7 +502,13 @@ private Q_SLOTS:
     void handleSslErrors(QNetworkReply *reply, const QList<QSslError> &errors);
 
 private:
-    void removeInternal(const ModelInfo &model);
+    // call with lock held
+    void updateDataInternal(const QString &id, const QVector<QPair<int, QVariant>> &data, QMutexLocker<QMutex> &lock,
+                            bool relock = true);
+
+    // call with lock held
+    void removeInternal(const QString &id, QMutexLocker<QMutex> &lock, bool relock = true);
+
     void clearDiscoveredModels();
     bool modelExists(const QString &fileName) const;
     int indexForModel(ModelInfo *model);

gpt4all-chat/src/mysettings.cpp

@@ -186,7 +186,7 @@ void MySettings::restoreModelDefaults(const ModelInfo &info)
     setModelTemperature(info, info.m_temperature);
     setModelTopP(info, info.m_topP);
     setModelMinP(info, info.m_minP);
-    setModelTopK(info, info.m_topK);;
+    setModelTopK(info, info.m_topK);
     setModelMaxLength(info, info.m_maxLength);
     setModelPromptBatchSize(info, info.m_promptBatchSize);
     setModelContextLength(info, info.m_contextLength);
@@ -226,9 +226,9 @@ void MySettings::restoreLocalDocsDefaults()
     setLocalDocsEmbedDevice(basicDefaults.value("localdocs/embedDevice").toString());
 }
 
-void MySettings::eraseModel(const ModelInfo &info)
+void MySettings::eraseModel(QStringView id)
 {
-    m_settings.remove(u"model-%1"_s.arg(info.id()));
+    m_settings.remove(u"model-%1"_s.arg(id));
 }
 
 QString MySettings::modelName(const ModelInfo &info) const

gpt4all-chat/src/mysettings.h

@@ -8,6 +8,7 @@
 #include <QSettings>
 #include <QString>
 #include <QStringList>
+#include <QStringView>
 #include <QTranslator>
 #include <QVector>
 
@@ -83,7 +84,7 @@ public:
     Q_INVOKABLE void restoreLocalDocsDefaults();
 
     // Model/Character settings
-    void eraseModel(const ModelInfo &info);
+    void eraseModel(QStringView id);
     QString modelName(const ModelInfo &info) const;
     Q_INVOKABLE void setModelName(const ModelInfo &info, const QString &name, bool force = false);
     QString modelFilename(const ModelInfo &info) const;