Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Retry" button #118

Closed
koppor opened this issue Aug 4, 2024 · 21 comments
Closed

Add "Retry" button #118

koppor opened this issue Aug 4, 2024 · 21 comments
Labels
Priority: normal Issue has normal priority status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue
Milestone

Comments

@koppor
Copy link
Collaborator

koppor commented Aug 4, 2024

Known from OpenAI UI. When something goes wrong, there is a "Retry" button.

JabRef currently just leaves one with the error message (here: Misal AI is used)

image

I would like to simply retry the message.

@InAnYan
Copy link
Owner

InAnYan commented Aug 5, 2024

I just don't really understand how the UI should work in these places.

What do you think about this approach:

When AI provider sends error, then:

  1. Text field and submit button are replaced with two buttons: "Retry" or "Cancel"
  2. On clicking "Retry", the error message disappears, UI changes to "waiting mode", and then we either get another error (go to step 1) or response from AI (restore text field and "submit")
  3. On clicking "Cancel" error message and last user message are gone, and message text field with "submit" button are restored

@InAnYan InAnYan added status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue and removed type: question labels Aug 5, 2024
@koppor
Copy link
Collaborator Author

koppor commented Aug 5, 2024

Sounds great!

@koppor koppor removed the status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue label Aug 5, 2024
@InAnYan
Copy link
Owner

InAnYan commented Aug 7, 2024

From Thi Lo: add a general retry button, to 1) regenerate response, 2) retry for error

InAnYan added a commit to JabRef/jabref that referenced this issue Aug 7, 2024
@InAnYan
Copy link
Owner

InAnYan commented Aug 7, 2024

I CAN'T BELIEVE it was so easy to make this feature.

Something, should be wrong...

So, it needs testing

@InAnYan InAnYan added the status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue label Aug 7, 2024
@koppor
Copy link
Collaborator Author

koppor commented Aug 7, 2024

I did not think through completely. Sorry.

"Cancel" should just cancel the retry. An activity "cancel" should not delete something.

JabRef should just return to the chat itself and have the system message there


Future work: Delete icons next to each message to delete a message

@koppor koppor removed the status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue label Aug 7, 2024
@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 7, 2024

Regenerate button did work.

Though, can we have a regenerate button for AI chat too? Would love to have that.

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 7, 2024

I think any potential "Cancel" button should cancel the generation, regardless if triggered by "regenerate" or "submit".

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

#118 (comment) Hmm, I see.

But then in the UI it will appear like this:

  • User: "sending message with error"
  • JabRef: "Error: ...."
  • User: "continue message"
  • JabRef: "Not error".

But in the chat history that would be stored like this:

  • User: "sending message with error"
  • User: "continue message"
  • JabRef: "Not error".

So, error message is disappeared, and there are 2 user messages in a row.

What should we do:

  1. "Cancel" will remain all messages, thus the situation I described will be like that
  2. "Cancel" will delete messages, like in current implementation --> leads to consistency (User-AI, AI-User, User-AI, etc.)
  3. Store errors in chat history

@InAnYan InAnYan added the status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue label Aug 9, 2024
@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

But then in the UI it will appear like this:

* User: "sending message with error"

* JabRef: "Error: ...."

* User: "continue message"

* JabRef: "Not error".

But in the chat history that would be stored like this: But then in the UI it will appear like this:

* User: "sending message with error"

* User: "continue message"

* JabRef: "Not error".

Ruslan, can you edit your message? It looks like you forgot to delete some parts or wanted to add some parts. In particular how the chat history would be stored. I think this is vital information about how to deal with this issue.

Depending on the info about how chat history is stored, my opinion might change, but currently, out of your options, I favor a combination of 1. and 3. IF we have delete icons next to each message to delete a message. Until we have delete icons, I am in favor of 2. for the time being.

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

Oh, sorry, I've just copy-pasted the text.

I hope you understood what I meant

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

Okay, after thinking, I think if user message question is gone, it's not cool. What if they want to reask it? Maybe they got wrong API key, and then they return back...

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

Users DO want the user message to be gone, if they want to have a different response.

Remember the "temperature" option in the preferences? That will allow you to have a different response each time you press the "retry" button (at least up to a certain degree. Of course, after a few dozens or hundred times of pressing the button, the responses will feel very similar, depending on your temperature settings)

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

Almost every AI GUI hast this button and they delete the last message upon retry. JabRef would be the outlier here.

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

Okay then, I understood.

I implement "Delete" button and do options 1 & 3

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

But then, what should we do after clicking "Cancel"?

Should we delete the user message too?

But then, we won't need 1 & 3

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

No. "Cancel" is a different feature. It is like a "Pause" or a "Break". Let's say the model is very verbose and starts to respond with a 10 000 tokens long response and it just doesn't stop, then pressing the "Cancel" button will allow you to stop the generation. It will also allow you to stop the generation, if you believe there is an error.

In this case, "deleting" the message is not wanted, because you may want to see what the model responded with. You may want to see the error message. You may want to see the start of the model's response, because having seen 200 out of 10 000 tokens might already have been enough and then users might want to copy and paste that part of the generation, etc.

TL;DR: I favor: Press "cancel" --> Stops generation, but does not delete the message.

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

@ThiloteE, what should "Cancel" button do on error from AI?

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

I am in favor of ""Retry" will delete messages --> leads to consistency (User-AI, AI-User, User-AI, etc.),if we can log the error messages somewhere else in JabRef and not in conversation, because I strongly believe disturbing users workflow with lots of errors and warnings is bad UI design (and it also looks ugly). But since it apparently only is possible to log the error messages in the conversations with langchain4j (at least that is what you said), deleting messages upon retry will lead to error messages getting lost, so that's not good either...

@ThiloteE
Copy link
Collaborator

ThiloteE commented Aug 9, 2024

@ThiloteE, what should "Cancel" button do on error from AI?

If there is an error message, then show the error message, unless we can log it somewhere else in JabRef. Please do not confuse "cancel" with "retry".

@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

@ThiloteE, I mean this issue. Okay, let's start like this:

  1. User sends message
  2. AI provider sends "Error: ..."

What should be done in this case?

  1. Should we just proceed?
  2. Or new UI should be added?

InAnYan added a commit to JabRef/jabref that referenced this issue Aug 9, 2024
@InAnYan
Copy link
Owner

InAnYan commented Aug 9, 2024

Closing, as implemented both "Retry" and "Cancel" buttons (the behavior of "Cancel" button was discussed with Thi Lo in a call, it won't delete any messages and it will just switch to default state)

@InAnYan InAnYan closed this as completed Aug 9, 2024
github-merge-queue bot pushed a commit to JabRef/jabref that referenced this issue Aug 14, 2024
* Fix the code from code review

* Fix from code review and create new AiChatTabWorking

* Improve chat history storage code

* More fix from code review

* Remove obsolete parameter

* Add JavaDoc comment

* Fix checkstyle

* Fix JavaDoc

* Fix more checkstyle

* More checkstyle fixes

* Fix code changes

* Improve the PR

* Rework ADR-0031 to enable to use another option

* Add many LOGGEr.trace statements

* Change "message window" to "context window"

* Fix compiler errors

* Fix issue list index issue of langchain4j

* Fix lint issue

* Update 0031-store-chats-alongside-database.md

* More tracing

* Refine logging

* Remove closing of AiChatLanguageModel (because it's not closable)

* Use external package for OpenAI API connection

* Provide a custom executor for RetrievalAugmentor

* Fix shutdown issue (I hope)

* Refactor classes

* Change BibDatabaseChatHistoryFile

* Revert BibDatabaseChatHistoryFile to old version because of langchain4j

* Make round corners for chat messages

* Refactor embeddings generation

* Refactor embeddings generation

* Refactor embeddings generation

* Fix CHANGELOG.md

* Remove jpro-mdfx

* Add comment

* Fix localizations

* Fix checkstyle and remove OpenAI from PRIVACY.md

* Remove unnecessary comments

* Fix privacy notice UI

* Introduce new ApiKeyMissingComponent

* Thanks Tobiaz Diez for writing such a good EntryEditorTab class

* Fix InAnYan/jabref issues

* Merge `build.gradle` and `settings.gradle` from main branch

* Update ADRs

* Implement rethought ADR for chat history

* Use OpenAI embedding model

* Use Deep Java embedding model

* Remove old langchain4j embedding models

* Fix checkstyle errors

* Fix checkstyle and remove old dependencies

* Fixes from code review

* Restructure

* Fix checkstyle errors

* Add API base URL parameter

* Fix localization

* Fix from code review + ADR

* Something broken

* Now MistralAI and Hugging Face work

* Fix base URL for other LLM providers

* Fix base URL for other LLM providers

* Refactor MVStore usage

* Load embedding model in background

* Bump langchain4j version

* Fix bug

* Fix checkstyle and localization

* Implement summarization

* Fix checkstyle and localization

* Improve PrivacyNoticeComponent

* Fix from code review

* Update localization

* Wrap text

* Add padding

* Fix markdown

* Use stuff algorithm

* Add GPT-4o-mini

* Make chat model editable

* Update context window size and summarization

* Fix checkstyle

* Update PrivacyNoticeComponent.fxml

* Update AI summary tab

* Fix localization

* Change order so that there is no diff

* Reorrder dependencies

* Add missing CHANGELOG.md entry

* Refine ADR-0033

* Refine ADR0034

* Fix typos

* Refine ADR-0036

* Fix ADR-0037

* Fix title case

* Fix changes in module-info.java

* Readd removed requires org.apache.httpcomponents.core5.httpcore5

* Revert change in JabRefGUI to avoid conflicts

* Remove empty lines

* Reorder entries in JabRef_en.properties

* Simplify SummariesStorage (and add test)

* Use region/endregion

* Fix position of comment

* Add comment why the event bus is needed

* Do not show exception to the user - just that an error is occurred (saves %0 in localization)

* Use "URL %0" without colon (consistency)

* Fix typos

* History has to be kept

* Remove empty lines

* Fix language (hopefully)

* Compilefix

* Simplify BibDatabaseChatHistoryManager

* Fix from code review

* Fix issue #103

* Rework embeddings cache clearing

* Fix #99 and partially #101

* Partially fixing shutdown issues and UI progress monitor issue

* Add "requires scala.library" and add "region:" / "endregion"

* More grouping (move de.saxsys.mvvmfx.validation up)

* Add alphabetical hint

* Fix InAnYan#101 and InAnYan#106

* Discard changes to settings.gradle

* Fix InAnYan#105

* Follow-up fix for InAnYan#103

* Follow-up fix for InAnYan#103

* Remove obsolete class

* Partially fix InAnYan#98

* We do need dependencies to the AI providers, don't we?

* Fix InAnYan#93

* Simplify code

* Partially fix InAnYan#92

* Fix checkstyle and localization

* Fix hyperlinks and text in ApiKeyMissingComponent

* Fixes from code review

* Fix InAnYan#120

* Remove "X% work done" messages

* Fix InAnYan#114

* Partially fix InAnYan#113

* Partially fix InAnYan#110

* Fix InAnYan#110

* Fix InAnYan#111

* Improve embedding model downloading notifications

* Fix InAnYan#124

* Fix InAnYan#122

* Fix wrong context window size when expert settings customization is turned off

* Attempt to fix InAnYan#95

* Finally fix InAnYan#105

* Fix InAnYan#108

* Attempt to fix InAnYan#98

* Fix for InAnYan#104

* Fix for InAnYan#98

* Fix for InAnYan#95 (comment)

* Fix for InAnYan#98 (comment)

* Fix for InAnYan#126

* Fix for InAnYan#115

* Fix for InAnYan#113

* Fix for InAnYan#91

* Fix for InAnYan#121

* Fix for InAnYan#112 and InAnYan#116

* Fix for InAnYan#125

* Fixes from commit comments

* Fix for InAnYan#115

* Fix for InAnYan#120

* Fix for InAnYan#132

* Fix for InAnYan#132

* Fix for InAnYan#104

* Fix for InAnYan#118

* Fix for InAnYan#114

* Fix for InAnYan#104

* Store error messages in chat history

* Make error be a ChatMessageComponent

* Implement delete messages InAnYan#136

* Fix for InAnYan#118

* Fix for InAnYan#92

* Fix checkstyle and localization. And refactoring

* Fix for InAnYan#92

* Fix for InAnYan#139

* Show "Delete message" button only when necessary

* Fix for InAnYan#83

* Update src/main/java/org/jabref/logic/ai/AiService.java

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

* Update src/main/java/org/jabref/logic/ai/chathistory/BibDatabaseChatHistoryManager.java

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

* Update src/main/java/org/jabref/logic/ai/AiService.java

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

* Update src/main/java/org/jabref/gui/Base.css

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

* Update src/main/java/org/jabref/gui/Base.css

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

* Fix from code review

* Partial fix for InAnYan#125

* Update colors for error message

* Fix for InAnYan#145 and InAnYan#142

* Make progress for embedding model download

* Fix checkstyle and localization

* Add workaround to get FileHistoryMenuTest running again

* Small fixes

* Revert "Small fixes"

This reverts commit 85382a1.

* Introduce AiApiKeyProvider

* Fix IDE setup instructions

* Do not load API keys on startup

* Rely on keystore encryption

* Prevent mulitple rebuilds when muliple preferences are updated

* Fix localization to be more provider independent

* Fix method names

* Add poor man's solution to notify of API key changes

* Reduce calls to key store (and fix key saving)

* Fix for InAnYan#148 and partially InAnYan#146

* Revert "Fix for InAnYan#148 and partially InAnYan#146"

This reverts commit 5fa3bb5.

* Fix for scrolling down when deleting a message

* Sort EmbeddingModel enum variants

* Fix GenerateSummaryTask progress indication

* Fix dark mode

* Add notice for embedding models size

---------

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: normal Issue has normal priority status: waiting-for-feedback The submitter needs to recheck the issue or provide more information about the issue
Projects
None yet
Development

No branches or pull requests

3 participants