Assistant design improvements + V2 API #3327
Conversation
…sistant-improvements
Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
|
@mpivchev I think AI conversations are rarely “completed”. People often want to ask follow-up questions. This aligns better with the natural flow of human reasoning (where follow-up questions are common), therefore, should avoid “completed”mark.
What is the difference between API V1 (<NC30) and API V2 (>=NC30)? Is there any improvements or difference from API V1? |
For now, however, since we just have input-output prompts, this is informative.
At the moment it looks like this: 
|
Thanks for the detailed info! It looks better and more useful now, with more functionalities. |
|
Hi @Hyeyoung346 the scrolling issue is already fixed in this PR. It's just open until this PR is merged :) |
|
Regarding 1. I'll see what I can do on Monday. Have a nice weekend 😀 |
…sistant-improvements
Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
…sistant-improvements
|
@Hyeyoung346 modified the date to show in a more human readable way 
I agree ultimately we should remove the status altogether and mimic a chat-like behavior. But for now, since it's just a prompt-style, we should keep it this way IMO. |
|
@jancborchardt can you quickly go over this :) |
There was a problem hiding this comment.
Looks like a really nice improvement @mpivchev! :)
I only have 2 small details for now, but they are not necessarily blockers, could also be done in follow-ups:
- The vertical space between the 3 lines in the task list should be even (right now there is more space above the date)
- I agree the "Completed" text could be cut. The icon is fine enough. And in the detail view, if it is still pending or if there is an error, that should show more presently as a sort of empty content view where the Output would normally be.
|
@mpivchev Seems do not use all width and the font seems a bit too big
|
…sistant-improvements
|
How do I test this? Either with our corporate production instance or the test server run by the shell script I have eternally scheduled tasks but never responses. |
…sistant-improvements
This doesnt happen for me 
|
|
I tried to test again but I cannot on our daily server due to nextcloud/server#51248 |
There was a problem hiding this comment.
@mpivchev can’t see your comment about the vertical spacing anymore, but if you think it’s too close together, let’s at least go for a padding value inbetween the current state and your screenshot. :)
Hi, added equal padding so it looks better now :) @jancborchardt 
|
Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
|
@mpivchev Is fixing the translation form out of scope? There is an obvious mismatch between the web version and what the app offers with its single text field and no hint what to enter there.
|
This is out of scope for now, currently iOS and Android only support certain tasks, and Translate is not one of them. In normal instances (not the one you use), I believe the API only returns the available tasks. |
There was a problem hiding this comment.
UI tests are fragile and fail for me. Mostly it is the login and not your fault. By now I think we should replace the fragile web view interaction by just injecting an app token through an app launch argument so the whole login can be skipped and tests run much faster to the point.
The tests did not work on the daily server, but tech preview somewhat better.
Also it was strange that I first had to remove all the unnecessary framework references from the Xcode UI test target. Xcode was complaining about RealmSwift not being found when linking. It is not even supposed to be in the UI test target. 😵💫
I stop at this point now, I could spend the rest of the day making the tests work.
| app.navigationBars["Assistant"].buttons["CreateButton"].tap() | ||
|
|
||
| app.textViews["InputTextEditor"].typeText(input) | ||
| app.navigationBars["New Free text to text prompt task"].buttons["Create"].tap() |
There was a problem hiding this comment.
The subscript is invalid. Issuing p app.navigationBars.allElementsBoundByAccessibilityElement.map(\.identifier) in the debugger prints:
([String]) 2 values {
[0] = "More"
[1] = "New ContextAgent task"
}
This resembles the navigation bar title "New ContextAgent task" in the sheet of the assistant.
There was a problem hiding this comment.
Interestingly, this is different when testing against the server on localhost:
([String]) 2 values {
[0] = "More"
[1] = "New Free text to text prompt task"
}
| try await aMoment() | ||
|
|
||
| let cell = app.collectionViews.children(matching: .cell).element(boundBy: 0) | ||
| XCTAssert(cell.staticTexts[taskInputCreated].exists) |
There was a problem hiding this comment.
This fails for me when testing against tech preview because "an error occurred" when creating the task, so there is no cell to find.
| createTask(input: taskInputCreated) | ||
|
|
||
| pullToRefresh() |
There was a problem hiding this comment.
When running against localhost, a short delay is needed in between, otherwise pull to refresh will fail because the cell has not appeared yet in the visible area.
The tests were made to be ran with Regarding injecting the app token, i think we did that before, let me check. |
Co-authored-by: Iva Horn <iva.horn@icloud.com> Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
I got confused because you recommended other servers. 😄 I see now, for manual testing only.
I think Talk team does it. |
Prerequisite: nextcloud/NextcloudKit#124
Some points @Hyeyoung346:
Server.shnow includes the commands to enable that, so you can start a server using that script.