feat(Assistant): implement full Assistant features also on client - #10345
feat(Assistant): implement full Assistant features also on client#10345Rello wants to merge 5 commits into
Conversation
|
The Assistant on the web will likely be redesigned next cycle, so I'd maybe hold off on iterating on the UI for now. |
Hello, |
|
@copilot resolve the merge conflicts in this pull request |
|
Merge conflicts with |
Yes.
That's fine by me. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97e91ad54d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Move Assistant operations into a dedicated controller and expose task and chat workflows in a separate window. Signed-off-by: Rello <Rello@users.noreply.github.com> Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
97e91ad to
26cdfe0
Compare
Remove the legacy tray implementation, encapsulate Assistant resources and networking, reject stale asynchronous replies, and bound chat polling. Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c66f53a706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
|
Artifact containing the AppImage: nextcloud-appimage-pr-10345.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
@copilot resolve the merge conflicts in this pull request |
Keep assistant module in src/gui/assistant/ with requestGeneration API. Remove assistant methods from usermodel.cpp (moved to assistant module). Remove src/libsync/ocsassistantconnector.cpp (moved to src/gui/assistant/). Assisted-by: ClaudeCode:claude-sonnet-4-5 Co-authored-by: Rello <13385119+Rello@users.noreply.github.com>
Merge conflicts with
|
|
Also seems like this doesn't build correctly @Rello maybe worth checking the agent product? |
Split large QML components, centralize UI measurements, align QML registration and Qt keyword usage, and cover chat status handling. Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ec87677a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| requestFailed(QStringLiteral("chatConversations"), statusCode); | ||
| return; | ||
| } | ||
| _chatConversations.replaceFromResponse(json, _selectedChatConversationId); |
There was a problem hiding this comment.
Unwrap OCS data before parsing chat responses
All added chat requests target /ocs/v2.php, and JsonApiJob delivers the complete OCS document, but this passes that object directly to AssistantConversationModel::replaceFromResponse(), which reads only json.array(). A normal { "ocs": { "data": [...] } } response therefore produces an empty conversation list; the create-message, generation, and session handlers make the same top-level assumption and consequently fail to find IDs or messages. Unwrap ocs.data consistently and cover these workflows with realistically enveloped responses.
AGENTS.md reference: AGENTS.md:L33-L36
Useful? React with 👍 / 👎.
|
claucambra
left a comment
There was a problem hiding this comment.
Some small changes and one bigger one. I think the test suite is insufficient for the breadth of additions being made here
| void slotChatSessionChecked(quint64 requestGeneration, const QJsonDocument &json, int statusCode); | ||
|
|
||
| private: | ||
| [[nodiscard]] quint64 beginRequest(); |
There was a problem hiding this comment.
Could we not just remove the [[nodiscard]] rather than casting to void wherever we don't need the result?
| return true; | ||
| }(); | ||
| Q_UNUSED(initialized) | ||
| } |
There was a problem hiding this comment.
What is the point of returning a value here?
| static void initializeAssistantResources() | ||
| { | ||
| Q_INIT_RESOURCE(assistant); | ||
| } |
There was a problem hiding this comment.
I'm not sure why we need this to be its own function? can't we just use it in the lambda below?
| void dateTextDoesNotGuessTimestampUnit() | ||
| { | ||
| constexpr auto timestamp = 1000000000001LL; | ||
| const auto expected = QLocale::system().toString(QDateTime::fromSecsSinceEpoch(timestamp), QLocale::ShortFormat); | ||
|
|
||
| QCOMPARE(AssistantUtils::dateText(timestamp), expected); | ||
| } |
There was a problem hiding this comment.
This test does not in actuality test anything useful; it's verifying a past implementation detail. Should be removed
|
|
||
| } | ||
|
|
||
| class TestAssistantController : public QObject |
There was a problem hiding this comment.
This test suite seems to be really limited compared to all the new additions




Uh oh!
There was an error while loading. Please reload this page.