fix: Found and fix review mistakes

2026-07-07 13:39:11 -04:00 · 2026-07-02 22:26:07 +02:00
parent c070d65366
commit 35bbaa1af0
139 changed files with 2032 additions and 1573 deletions
--- a/sources/Session/ContextAssembler.cpp
+++ b/sources/Session/ContextAssembler.cpp
@@ -22,9 +22,12 @@ Q_LOGGING_CATEGORY(ctxLog, "qodeassist.context")
 QString roleToWireString(Message::Role role)
 {
    switch (role) {
-    case Message::Role::System: return QStringLiteral("system");
-    case Message::Role::User: return QStringLiteral("user");
-    case Message::Role::Assistant: return QStringLiteral("assistant");
+    case Message::Role::System:
+        return QStringLiteral("system");
+    case Message::Role::User:
+        return QStringLiteral("user");
+    case Message::Role::Assistant:
+        return QStringLiteral("assistant");
    }
    return QStringLiteral("user");
 }
@@ -70,7 +73,9 @@ QString Manifest::summary() const
    if (unsupportedBlocks > 0)
        s += QStringLiteral(", unsupported=%1").arg(unsupportedBlocks);
    if (!elided.isEmpty())
-        s += QStringLiteral(", elided=%1 [%2]").arg(elided.size()).arg(elided.join(QStringLiteral("; ")));
+        s += QStringLiteral(", elided=%1 [%2]")
+                 .arg(elided.size())
+                 .arg(elided.join(QStringLiteral("; ")));
    return s;
 }

@@ -135,8 +140,7 @@ Templates::ContextData assemble(
                e.kind = ContentBlockEntry::Kind::Image;
                e.imageData = img->data();
                e.mediaType = img->mediaType();
-                e.isImageUrl
-                    = (img->sourceType() == LLMQore::ImageContent::ImageSourceType::Url);
+                e.isImageUrl = (img->sourceType() == LLMQore::ImageContent::ImageSourceType::Url);
                blockEntries.append(std::move(e));
                ++manifest.imageBlocks;
            } else if (auto *si = dynamic_cast<StoredImageContent *>(block)) {
@@ -144,8 +148,7 @@ Templates::ContextData assemble(
                if (base64.isEmpty()) {
                    blockEntries.append(
                        makeTextEntry(placeholderFor(QStringLiteral("Image"), si->fileName())));
-                    manifest.elided
-                        << QStringLiteral("image unavailable: %1").arg(si->fileName());
+                    manifest.elided << QStringLiteral("image unavailable: %1").arg(si->fileName());
                    qCWarning(ctxLog).noquote()
                        << "stored image unavailable, placeholder inserted:" << si->fileName();
                    continue;
@@ -165,8 +168,7 @@ Templates::ContextData assemble(
                    manifest.elided
                        << QStringLiteral("attachment unavailable: %1").arg(sa->fileName());
                    qCWarning(ctxLog).noquote()
-                        << "stored attachment unavailable, placeholder inserted:"
-                        << sa->fileName();
+                        << "stored attachment unavailable, placeholder inserted:" << sa->fileName();
                    continue;
                }
                const QString text = QString::fromUtf8(QByteArray::fromBase64(stored.toUtf8()));
@@ -199,8 +201,7 @@ Templates::ContextData assemble(
                blockEntries.append(std::move(e));
            } else if (auto *tu = dynamic_cast<LLMQore::ToolUseContent *>(block)) {
                if (!resolvedToolUseIds.contains(tu->id())) {
-                    manifest.elided
-                        << QStringLiteral("orphan tool_use dropped: %1").arg(tu->id());
+                    manifest.elided << QStringLiteral("orphan tool_use dropped: %1").arg(tu->id());
                    continue;
                }
                ContentBlockEntry e;
@@ -233,11 +234,11 @@ Templates::ContextData assemble(
        if (blockEntries.isEmpty())
            continue;

-        const bool hasNonThinking = std::any_of(
-            blockEntries.begin(), blockEntries.end(), [](const ContentBlockEntry &e) {
-                return e.kind != ContentBlockEntry::Kind::Thinking
-                       && e.kind != ContentBlockEntry::Kind::RedactedThinking;
-            });
+        const bool hasNonThinking
+            = std::any_of(blockEntries.begin(), blockEntries.end(), [](const ContentBlockEntry &e) {
+                  return e.kind != ContentBlockEntry::Kind::Thinking
+                         && e.kind != ContentBlockEntry::Kind::RedactedThinking;
+              });
        if (!hasNonThinking) {
            manifest.elided << QStringLiteral("thinking-only message dropped");
            continue;
--- a/sources/Session/ConversationHistory.cpp
+++ b/sources/Session/ConversationHistory.cpp
@@ -33,7 +33,7 @@ void ConversationHistory::appendTextDeltaToLast(const QString &delta)
        return;

    auto &last = m_messages.back();
-    if (auto *text = last.lastBlockOfType<LLMQore::TextContent>()) {
+    if (auto *text = dynamic_cast<LLMQore::TextContent *>(last.lastBlock())) {
        text->appendText(delta);
    } else {
        last.appendBlock(std::make_unique<LLMQore::TextContent>(delta));
@@ -41,22 +41,16 @@ void ConversationHistory::appendTextDeltaToLast(const QString &delta)
    emit messageUpdated(static_cast<int>(m_messages.size()) - 1);
 }

-void ConversationHistory::appendThinkingDeltaToLast(const QString &delta, const QString &signature)
+void ConversationHistory::appendThinkingBlockToLast(const QString &thinking, const QString &signature)
 {
-    if (m_messages.empty() || (delta.isEmpty() && signature.isEmpty()))
+    if (m_messages.empty() || (thinking.isEmpty() && signature.isEmpty()))
        return;

    auto &last = m_messages.back();
-    auto *thinking = last.lastBlockOfType<LLMQore::ThinkingContent>();
-    if (!thinking) {
-        auto fresh = std::make_unique<LLMQore::ThinkingContent>(delta, signature);
-        last.appendBlock(std::move(fresh));
-    } else {
-        if (!delta.isEmpty())
-            thinking->appendThinking(delta);
-        if (!signature.isEmpty())
-            thinking->setSignature(signature);
-    }
+    if (thinking.isEmpty())
+        last.appendBlock(std::make_unique<LLMQore::RedactedThinkingContent>(signature));
+    else
+        last.appendBlock(std::make_unique<LLMQore::ThinkingContent>(thinking, signature));
    emit messageUpdated(static_cast<int>(m_messages.size()) - 1);
 }

--- a/sources/Session/ConversationHistory.hpp
+++ b/sources/Session/ConversationHistory.hpp
@@ -32,7 +32,7 @@ public:
    void appendBlockToLast(std::unique_ptr<LLMQore::ContentBlock> block);

    void appendTextDeltaToLast(const QString &delta);
-    void appendThinkingDeltaToLast(const QString &delta, const QString &signature = QString());
+    void appendThinkingBlockToLast(const QString &thinking, const QString &signature = QString());

    void clear();
    void resetTo(int index);
--- a/sources/Session/ErrorInfo.hpp
+++ b/sources/Session/ErrorInfo.hpp
@@ -43,14 +43,14 @@ struct ErrorInfo
        return text.contains(QLatin1String(needle));
    };

-    if (contains("401") || contains("403") || contains("unauthorized")
-        || contains("forbidden") || contains("api key") || contains("apikey")
-        || contains("authentication") || contains("invalid token"))
+    if (contains("401") || contains("403") || contains("unauthorized") || contains("forbidden")
+        || contains("api key") || contains("apikey") || contains("authentication")
+        || contains("invalid token"))
        return ErrorCategory::Auth;

    if (contains("timeout") || contains("timed out") || contains("connection")
-        || contains("could not resolve") || contains("unreachable")
-        || contains("network") || contains("ssl") || contains("refused"))
+        || contains("could not resolve") || contains("unreachable") || contains("network")
+        || contains("ssl") || contains("refused"))
        return ErrorCategory::Network;

    return ErrorCategory::Provider;
--- a/sources/Session/Message.hpp
+++ b/sources/Session/Message.hpp
@@ -45,6 +45,16 @@ public:
            m_blocks.push_back(std::move(block));
    }

+    LLMQore::ContentBlock *lastBlock() noexcept
+    {
+        return m_blocks.empty() ? nullptr : m_blocks.back().get();
+    }
+
+    std::vector<std::unique_ptr<LLMQore::ContentBlock>> takeBlocks() noexcept
+    {
+        return std::move(m_blocks);
+    }
+
    template<typename T>
    T *lastBlockOfType()
    {
--- a/sources/Session/MessageSerializer.cpp
+++ b/sources/Session/MessageSerializer.cpp
@@ -165,7 +165,7 @@ std::unique_ptr<LLMQore::ContentBlock> blockFromJson(const QJsonObject &obj)
            FileEditContent::statusFromString(obj.value("status").toString()),
            obj.value("statusMessage").toString());
    }
-    return nullptr; // unknown type — skipped
+    return nullptr;
 }

 } // namespace
@@ -179,8 +179,11 @@ QJsonObject MessageSerializer::toJson(const Message &message)

    QJsonArray blocks;
    for (const auto &b : message.blocks()) {
-        if (b)
-            blocks.append(blockToJson(*b));
+        if (!b)
+            continue;
+        const QJsonObject blockObj = blockToJson(*b);
+        if (!blockObj.isEmpty())
+            blocks.append(blockObj);
    }
    obj["blocks"] = blocks;
    return obj;
--- a/sources/Session/ResponseEvent.hpp
+++ b/sources/Session/ResponseEvent.hpp
@@ -144,8 +144,7 @@ public:
            ResponseEvents::Usage{inputTokens, outputTokens, cachedTokens, reasoningTokens}};
    }

-    static ResponseEvent error(
-        QString message, ErrorCategory category = ErrorCategory::Provider)
+    static ResponseEvent error(QString message, ErrorCategory category = ErrorCategory::Provider)
    {
        return {Kind::Error, ResponseEvents::Error{std::move(message), category}};
    }
--- a/sources/Session/ResponseRouter.cpp
+++ b/sources/Session/ResponseRouter.cpp
@@ -102,7 +102,7 @@ void ResponseRouter::onThinking(
        return;
    ensureAssistantOpen();
    if (m_history)
-        m_history->appendThinkingDeltaToLast(thinking, signature);
+        m_history->appendThinkingBlockToLast(thinking, signature);
    emit event(ResponseEvent::thinkingDelta(thinking, signature));
 }

@@ -153,11 +153,12 @@ void ResponseRouter::onFinalized(
    if (id != m_activeId)
        return;
    if (info.usage) {
-        emit event(ResponseEvent::usage(
-            info.usage->promptTokens,
-            info.usage->completionTokens,
-            info.usage->cachedPromptTokens,
-            info.usage->reasoningTokens));
+        emit event(
+            ResponseEvent::usage(
+                info.usage->promptTokens,
+                info.usage->completionTokens,
+                info.usage->cachedPromptTokens,
+                info.usage->reasoningTokens));
    }
    emit event(ResponseEvent::messageStop(info.stopReason));
    endRequest();
--- a/sources/Session/Session.cpp
+++ b/sources/Session/Session.cpp
@@ -28,13 +28,14 @@ namespace {
 } // namespace

 Session::Session(Agent *agent, QObject *parent)
-    : Session(agent, /*externalHistory=*/nullptr, parent)
+    : Session(agent, nullptr, parent)
 {}

 Session::Session(Agent *agent, ConversationHistory *externalHistory, QObject *parent)
    : QObject(parent)
    , m_history(externalHistory ? externalHistory : new ConversationHistory(this))
    , m_systemPrompt(new SystemPromptBuilder(this))
+    , m_externalHistory(externalHistory != nullptr)
 {
    if (agent)
        setAgent(agent);
@@ -45,15 +46,16 @@ void Session::setAgent(Agent *agent)
    if (agent == m_agent)
        return;

-    if (isInFlight())
-        teardownInFlight();
+    cancel();

    if (m_router) {
-        delete m_router;
+        m_router->disconnect(this);
+        m_router->deleteLater();
        m_router = nullptr;
    }

-    delete m_agent;
+    if (m_agent)
+        m_agent->deleteLater();
    m_agent = agent;
    m_invalidReason.clear();

@@ -84,8 +86,7 @@ void Session::setAgent(Agent *agent)

 Session::~Session()
 {
-    if (isInFlight())
-        teardownInFlight();
+    cancel();
 }

 bool Session::isValid() const noexcept
@@ -154,6 +155,11 @@ void Session::unpinContext(const QString &id)
    std::erase_if(m_pinnedProviders, [&id](const auto &entry) { return entry.first == id; });
 }

+void Session::clearPinnedContext()
+{
+    m_pinnedProviders.clear();
+}
+
 LLMQore::RequestID Session::send(std::vector<std::unique_ptr<LLMQore::ContentBlock>> userBlocks)
 {
    if (!canSend()) {
@@ -165,19 +171,24 @@ LLMQore::RequestID Session::send(std::vector<std::unique_ptr<LLMQore::ContentBlo
        return {};
    }
    if (userBlocks.empty() || !m_history) {
-        m_lastError = makeError(ErrorCategory::Validation, QStringLiteral("Session: nothing to send"));
+        m_lastError
+            = makeError(ErrorCategory::Validation, QStringLiteral("Session: nothing to send"));
        return {};
    }

    if (isInFlight())
        cancel();

+    const int preSendSize = m_history->size();
    Message msg(Message::Role::User);
    for (auto &b : userBlocks)
        msg.appendBlock(std::move(b));
    m_history->append(std::move(msg));

-    return dispatch();
+    const auto id = dispatch();
+    if (id.isEmpty())
+        m_history->resetTo(preSendSize);
+    return id;
 }

 QVector<ContextAssembler::PinnedBlock> Session::materializePinned() const
@@ -223,8 +234,8 @@ LLMQore::RequestID Session::dispatch()
        m_systemPrompt->clearLayer(QStringLiteral("agent.system"));
    } else {
        QString renderErr;
-        const QString renderedContext = Templates::ContextRenderer::render(
-            cfg.systemPrompt, m_contextBindings, &renderErr);
+        const QString renderedContext
+            = Templates::ContextRenderer::render(cfg.systemPrompt, m_contextBindings, &renderErr);
        if (!renderErr.isEmpty()) {
            m_lastError = makeError(
                ErrorCategory::Validation,
@@ -237,7 +248,9 @@ LLMQore::RequestID Session::dispatch()
            m_systemPrompt->clearLayer(QStringLiteral("agent.system"));
        else
            m_systemPrompt->setLayer(
-                QStringLiteral("agent.system"), renderedContext, SystemPromptBuilder::kAgentPriority);
+                QStringLiteral("agent.system"),
+                renderedContext,
+                SystemPromptBuilder::kAgentPriority);
    }

    return dispatchContext(assembleContext(), cfg.enableTools);
@@ -259,13 +272,37 @@ LLMQore::RequestID Session::dispatchContext(const Templates::ContextData &ctx, b

    QString endpoint = cfg.endpoint;
    endpoint.replace(QStringLiteral("${MODEL}"), cfg.model);
+
+    LLMQore::RequestID earlyFailId;
+    QString earlyFailError;
+    QMetaObject::Connection earlyFailConn;
+    if (auto *cl = client()) {
+        earlyFailConn = connect(
+            cl,
+            &LLMQore::BaseClient::requestFailed,
+            this,
+            [&earlyFailId, &earlyFailError](const LLMQore::RequestID &fid, const QString &err) {
+                earlyFailId = fid;
+                earlyFailError = err;
+            },
+            Qt::DirectConnection);
+    }
+
    const auto id = provider->sendRequest(QUrl(provider->url()), payload, endpoint);
+
+    if (earlyFailConn)
+        disconnect(earlyFailConn);
+
    if (id.isEmpty()) {
        m_lastError = makeError(
            ErrorCategory::Provider,
            QStringLiteral("Provider '%1' failed to start the request").arg(provider->name()));
        return {};
    }
+    if (id == earlyFailId && !earlyFailError.isEmpty()) {
+        m_lastError = makeError(categorizeProviderError(earlyFailError), earlyFailError);
+        return {};
+    }

    m_inFlight = id;
    if (m_router)
@@ -274,8 +311,7 @@ LLMQore::RequestID Session::dispatchContext(const Templates::ContextData &ctx, b
    return id;
 }

-QJsonObject Session::buildPayload(
-    const Templates::ContextData &ctx, bool tools, QString *errOut) const
+QJsonObject Session::buildPayload(const Templates::ContextData &ctx, bool tools, QString *errOut) const
 {
    auto *provider = m_agent->provider();
    auto *tmpl = m_agent->promptTemplate();
@@ -302,7 +338,7 @@ Templates::ContextData Session::assembleContext() const
 void Session::onRouterEvent(const ResponseEvent &ev)
 {
    if (m_inFlight.isEmpty())
-        return; // stale events after cancel
+        return;

    emit event(ev);

--- a/sources/Session/Session.hpp
+++ b/sources/Session/Session.hpp
@@ -56,10 +56,12 @@ public:
    using PinnedProvider = std::function<QString()>;
    void pinContext(const QString &id, PinnedProvider provider);
    void unpinContext(const QString &id);
+    void clearPinnedContext();

    Agent *agent() noexcept { return m_agent; }
    void setAgent(Agent *agent);
    ConversationHistory *history() const noexcept { return m_history; }
+    bool usesExternalHistory() const noexcept { return m_externalHistory; }
    SystemPromptBuilder *systemPrompt() const noexcept { return m_systemPrompt; }

    LLMQore::BaseClient *client() const noexcept;
@@ -89,10 +91,11 @@ private:
    QVector<ContextAssembler::PinnedBlock> materializePinned() const;
    QJsonObject buildPayload(const Templates::ContextData &ctx, bool tools, QString *errOut) const;

-    Agent *m_agent = nullptr;                              // child if non-null
-    QPointer<ConversationHistory> m_history;               // child if internal, external otherwise
-    SystemPromptBuilder *m_systemPrompt = nullptr; // child
-    ResponseRouter *m_router = nullptr;                    // child, only when valid
+    Agent *m_agent = nullptr;
+    QPointer<ConversationHistory> m_history;
+    SystemPromptBuilder *m_systemPrompt = nullptr;
+    ResponseRouter *m_router = nullptr;
+    bool m_externalHistory = false;

    LLMQore::RequestID m_inFlight;
    QString m_invalidReason;
--- a/sources/Session/SessionManager.cpp
+++ b/sources/Session/SessionManager.cpp
@@ -27,7 +27,7 @@ SessionManager::~SessionManager() = default;

 Session *SessionManager::createSession(const QString &agentName, QString *errorOut)
 {
-    return createSession(agentName, /*externalHistory=*/nullptr, errorOut);
+    return createSession(agentName, nullptr, errorOut);
 }

 Session *SessionManager::createSession(
@@ -40,7 +40,7 @@ Session *SessionManager::createSession(
    }

    QString agentErr;
-    Agent *agent = m_agentFactory->create(agentName, /*parent=*/nullptr, &agentErr);
+    Agent *agent = m_agentFactory->create(agentName, nullptr, &agentErr);
    if (!agent) {
        if (errorOut)
            *errorOut = agentErr.isEmpty()
@@ -53,7 +53,7 @@ Session *SessionManager::createSession(
    if (!session->isValid()) {
        if (errorOut)
            *errorOut = session->invalidReason();
-        delete session; // also deletes the reparented agent
+        delete session;
        return nullptr;
    }

@@ -64,7 +64,7 @@ Session *SessionManager::createSession(

 Session *SessionManager::createDetachedSession(ConversationHistory *externalHistory, QObject *parent)
 {
-    return new Session(/*agent=*/nullptr, externalHistory, parent);
+    return new Session(nullptr, externalHistory, parent);
 }

 bool SessionManager::rebindAgentByName(Session *session, const QString &agentName, QString *errorOut)
@@ -81,7 +81,7 @@ bool SessionManager::rebindAgentByName(Session *session, const QString &agentNam
    }

    QString agentErr;
-    Agent *agent = m_agentFactory->create(agentName, /*parent=*/nullptr, &agentErr);
+    Agent *agent = m_agentFactory->create(agentName, nullptr, &agentErr);
    if (!agent) {
        if (errorOut)
            *errorOut = agentErr.isEmpty()
@@ -113,7 +113,7 @@ Session *SessionManager::acquire(const QString &agentName, QString *errorOut)
            pooled->deleteLater();
    }

-    return createSession(agentName, /*externalHistory=*/nullptr, errorOut);
+    return createSession(agentName, nullptr, errorOut);
 }

 void SessionManager::release(Session *session)
@@ -130,10 +130,16 @@ void SessionManager::release(Session *session)
        session->cancel();

    session->disconnect();
+
+    if (session->usesExternalHistory()) {
+        emit sessionRemoved(session);
+        session->deleteLater();
+        return;
+    }
+
    resetSession(session);

-    const QString agentName
-        = session->agent() ? session->agent()->config().name : QString();
+    const QString agentName = session->agent() ? session->agent()->config().name : QString();
    QList<QPointer<Session>> &bucket = m_pool[agentName];
    if (agentName.isEmpty() || bucket.size() >= kMaxPooledPerAgent) {
        emit sessionRemoved(session);
@@ -155,6 +161,9 @@ void SessionManager::resetSession(Session *session)
        if (auto *tools = client->tools())
            tools->removeAllTools();
    }
+    session->setContentLoader({});
+    session->setContextBindings({});
+    session->clearPinnedContext();
 }

 bool SessionManager::pooledAgentMatchesCurrent(Session *session, const QString &agentName) const
--- a/sources/Session/SessionManager.hpp
+++ b/sources/Session/SessionManager.hpp
@@ -53,8 +53,7 @@ signals:
 private:
    void resetSession(Session *session);
    void flushPool();
-    [[nodiscard]] bool pooledAgentMatchesCurrent(
-        Session *session, const QString &agentName) const;
+    [[nodiscard]] bool pooledAgentMatchesCurrent(Session *session, const QString &agentName) const;

    static constexpr int kMaxPooledPerAgent = 2;

--- a/sources/Session/SystemPromptBuilder.cpp
+++ b/sources/Session/SystemPromptBuilder.cpp
@@ -49,7 +49,8 @@ void SystemPromptBuilder::clear()
 QString SystemPromptBuilder::layer(const QString &name) const
 {
    for (const auto &l : m_layers) {
-        if (l.name == name) return l.text;
+        if (l.name == name)
+            return l.text;
    }
    return {};
 }
@@ -58,16 +59,17 @@ QStringList SystemPromptBuilder::layerNames() const
 {
    QStringList out;
    out.reserve(m_layers.size());
-    for (const auto &l : m_layers) out.append(l.name);
+    for (const auto &l : m_layers)
+        out.append(l.name);
    return out;
 }

 QString SystemPromptBuilder::compose(const QString &separator) const
 {
    QVector<Layer> ordered = m_layers;
-    std::stable_sort(
-        ordered.begin(), ordered.end(),
-        [](const Layer &a, const Layer &b) { return a.priority < b.priority; });
+    std::stable_sort(ordered.begin(), ordered.end(), [](const Layer &a, const Layer &b) {
+        return a.priority < b.priority;
+    });

    QStringList parts;
    parts.reserve(ordered.size());