Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Loading Smarti fails for empty conversations #258

Closed
ruKurz opened this issue Jun 12, 2018 · 24 comments
Closed

Loading Smarti fails for empty conversations #258

ruKurz opened this issue Jun 12, 2018 · 24 comments
Assignees
Milestone

Comments

@ruKurz
Copy link
Collaborator

ruKurz commented Jun 12, 2018

Loading the Smarti Widget fails when the conversation is empty:

Expected behavior

If a new "topic", "help-request", "thread" is started in Assistify.Chat that does not have any messages in it yet. The smarti widget should inform the user that the "Knowledge Base" is working, but that there are no messages within the room, that can be analyzed.

Actual behavior

The Smarti Widget show an error message that is misleading. It suggests that there is some problem with the knowledge base, but actually everything went well and there are just being no results to be displayed yet.
smarti not loaded

Steps to reproduce

  1. Create a new topic without an initial message

Cause assumption

When creating a new topic there is printed out an error into the Smarti server log. It seems as there is a valid analysis coming back, but instead of checking if the client gets an valid Solr query, the client purely tries to execute a similarity search without any "q" or "q.alt" parameters present.

2018-06-12 16:22:02.732 ERROR 3665 --- [pool-1-thread-1] o.a.solr.handler.RequestHandlerBase      : org.apache.solr.common.SolrException: MoreLikeThis requires either a query (?q=) or text to find similar documents.
	at org.apache.solr.handler.MoreLikeThisHandler.handleRequestBody(MoreLikeThisHandler.java:195)
	at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:173)
	at org.apache.solr.core.SolrCore.execute(SolrCore.java:2477)
	at org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.request(EmbeddedSolrServer.java:179)
	at org.apache.solr.client.solrj.SolrClient.request(SolrClient.java:1219)
	at io.redlink.smarti.query.conversation.ConversationSearchQueryBuilder.buildContextQuery(ConversationSearchQueryBuilder.java:261)
	at io.redlink.smarti.query.conversation.ConversationSearchQueryBuilder.buildQuery(ConversationSearchQueryBuilder.java:188)
	at io.redlink.smarti.query.conversation.ConversationSearchQueryBuilder.buildQuery(ConversationSearchQueryBuilder.java:56)
	at io.redlink.smarti.query.conversation.ConversationQueryBuilder.doBuildQuery(ConversationQueryBuilder.java:95)
	at io.redlink.smarti.api.QueryBuilder.lambda$buildQuery$4(QueryBuilder.java:111)
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
	at io.redlink.smarti.api.QueryBuilder.buildQuery(QueryBuilder.java:109)
	at io.redlink.smarti.services.QueryBuilderService.buildQueries(QueryBuilderService.java:107)
	at io.redlink.smarti.services.QueryBuilderService.buildQueries(QueryBuilderService.java:76)
	at io.redlink.smarti.services.AnalysisService.lambda$process$1(AnalysisService.java:272)
	at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1590)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:748)
@Peym4n
Copy link
Contributor

Peym4n commented Jun 12, 2018

@ruKurz Could you please provide some steps to reproduce it?

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 12, 2018

@Peym4n In updated the description of this issue. Does that fir fore you?

@Peym4n
Copy link
Contributor

Peym4n commented Jun 12, 2018

I'm not able to reproduce the Smarti error by creating a new empty request.
But I was able to reproduce it by creating a request and then deleting the initial message in it.
After RC sends the delete request to Smarti, that error happens. Without the widget sending any requests.
So, I see two issues here. One is the widget not showing an appropriate message when the conversation is empty. And the other one is the Smarti error when deleting the initial message. @ruKurz Could you please create a new issue for the latter?

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 12, 2018

@Peym4n what does happen if you're creating a new topic?

@Peym4n
Copy link
Contributor

Peym4n commented Jun 12, 2018

After creating a topic (request) with no initial message, I just get the widget notification that is shown above, but no errors in Smarti.

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 12, 2018

I'll recheck with an empty MongoDB, just to assure that it's no "works not on my machine" problem. And come back soon.

Just a remark: A topic != request. Did you really create an topic?

@Peym4n
Copy link
Contributor

Peym4n commented Jun 12, 2018

Did you really create an topic?

Yes, I tried creating a new topic as well. And the result was the same. I get the widget notification but no Smarti error.

btw, in the steps to reproduce it says,

Create a new topic without an initial message

But a topic cannot have an initial message, just a request can. That's why I thought you are talking about creating a new request with no initial message.

@Peym4n
Copy link
Contributor

Peym4n commented Jun 12, 2018

I'm using our smarti-widget-0.7.4 branch for RC. And the develop branch of Smarti.

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 12, 2018

One is the widget not showing an appropriate message when the conversation is empty.

Right!

And the other one is the Smarti error when deleting the initial message.

The error seems to occure each time an analysis for an empty conversation is returned from the Smarti server to the widget.

@ruKurz Could you please create a new issue for the latter?

Since I get the error without deleting a RC message or calling DELETE on Smarti API I do not see a relation between this issue and the deletion. Before creating a new issue, I want to validate that this error is not caused by an empty conversation/analysis result.

Background Until now we did not create a conversation when a new topic has been created.
We're currently working on assistify/Rocket.Chat#346
By this PR an empty Smarti conversation will be created when creating a new topic or an request without initial message.

Smarti: 16fc08d

Assistify.Chat: assistify/Rocket.Chat@43fe233

@Peym4n could you please use these versions for testing?

@westei
Copy link
Member

westei commented Jun 13, 2018

I will fix the reported Exception by not performing an actual query if the context is an empty text but rather assume an empty result - resulting in an empty q.alt. An empty q.altwill cause an empty response for Expertengespräche what is anyways the desired result

@westei westei self-assigned this Jun 13, 2018
@westei westei added this to the v0.7.4 milestone Jun 13, 2018
westei added a commit that referenced this issue Jun 13, 2018
* now `""` is returned in case of an empty context. As this will be used as `q.alt` the fallback query will not return any results
* An empty context query is now also returned in case a SolrException is thrown while building the context query.
@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 13, 2018

@Peym4n if @westei fixes the server issue, would you improve the info message displayed when no Smarti results have been found (from error to info)? That would be great!

@westei
Copy link
Member

westei commented Jun 13, 2018

@ruKurz my change only fixed the SolrException, but not that Channels without a Message are not available in Smarti.

After checking with the MongoDB the Server-side code and the Rocket.Chat integration the behaviour originates from the fact that:

The Rocket.Chat integration only creates a Smarti Conversation when a Message is sent for a Channel. Channel create/delete events are not processed.
The Smarti Conversation is currently created when the Rocket.Chat integration receives a Message for a Channel that is not yet known by Smarti.

Because of this Smarti shows the Information that this room is not available in the knowledge base.

To fully solve this we would need to adapt the integration to also react on Channel level events

@Peym4n
Copy link
Contributor

Peym4n commented Jun 13, 2018

@Peym4n could you please use these versions for testing?

I started RC and Smarti using the commits that you provided.
And now I see the error. It happens after a POST request from RC to Smarti's conversation endpoint with no messages. So it has nothing to do with the Widget.

@Peym4n
Copy link
Contributor

Peym4n commented Jun 13, 2018

@ruKurz ok. I will change the widget notification to info, when no conversation is found.

@westei
Copy link
Member

westei commented Jun 13, 2018

@Peym4n I installed the Smarti develop and Rocket.Chat https://github.com/assistify/Rocket.Chat/tree/fix/smarti-results-not-loaded on the test server and was able to verify that no Smarti Conversation is created when a new Channel is created in Rocket.Chat. Only when the first Message is sent to the Channel the Conversation + Message are created with Smarti.

This is the reason for the Error message as shown in this Issue

@Peym4n
Copy link
Contributor

Peym4n commented Jun 13, 2018

@ruKurz We found out that when afterCreateChannel and _createAndPostConversation are called, an empty conversation is created but the conversation ID is not stored in the mapping, that's why the widget cannot find the new conversation, until the first message is sent. Is that expected?

And can this issue be in review now? or are any more changes from our side needed?

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 13, 2018

We found out that when afterCreateChannel and _createAndPostConversation are called, an empty conversation is created but the conversation ID is not stored in the mapping, that's why the widget cannot find the new conversation, until the first message is sent. Is that expected?

  1. afterCreateChannel: @ThomasRoehl has also figured out this behavior. It is already fixed: assistify/Rocket.Chat@dcfa1e0
  2. handle empty q.alt: @westei has fixed the server error: c2178a1
  3. info message: @Peym4n has fixed the info message: 97353c4

And can this issue be in review now?

So from my point of view we can move this issue into review.

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 13, 2018

@Peym4n I tested the info message. It's now in another color. But the info text is semantically not correct. It says: "Dieser Raum ist im Wissensbasis nicht verfügbar."

A semantically correct message would be: "Starte die Unterhaltung, damit Vorschläge aus der Wissensbasis vorgeschlagen werden."

BTW: The sentence "Dieser Raum ist im Wissensbasis nicht verfügbar." is grammatically incorrect.

@westei
Copy link
Member

westei commented Jun 14, 2018

Hi Rüdiger

With the following environment:

  • Smarti v0.7.4
  • Rocket.Chat https://github.com/assistify/Rocket.Chat.git/fix/smarti-results-not-loaded

and the following process:

  1. Kanal Anlegen
  2. Bestehendes Thema auswählen (alles andere leer lassen)
  3. Anlegen Button drücken

results for me in the state as shown in the following screenshot

Screen Shot 2018-06-14 at 09.46.46.PNG

I do not see any message that the channel is not available in the knowledge base.

In my opinion this is the expected behaviour.

Unrelated to this we should fix the info message to be grammatically correct

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 14, 2018

On our IAT we did not yet install https://github.com/assistify/Rocket.Chat.git/fix/smarti-results-not-loaded. So we will retest hopefully this day.

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 14, 2018

Regarding the "info". We must handle several cases:

  1. There is no conversation for room (the current message)
  2. There is no message in the room and therefore no results can be found (no similar messages found)
  3. Smarti is effectively not reachable or does not work for any other reasons (knowledge base is unavailable)

I would like to see this improvement within the current release.
So what do you prefer? Should we create a new issue for that, or is it fine for you to solve that within this issue?

@Peym4n
Copy link
Contributor

Peym4n commented Jun 14, 2018

@ruKurz I'm working on it. But could you please create a new issue for that? Just to keep things separated.

@westei
Copy link
Member

westei commented Jun 14, 2018

@Peym4n @ruKurz created #261 for. Move this back to done (as it is part of v0.7.4

@ruKurz
Copy link
Collaborator Author

ruKurz commented Jun 14, 2018

#262

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants