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

Improve @ForIntent Annotation #31

Open
cbeaujoin opened this issue Sep 9, 2019 · 13 comments
Open

Improve @ForIntent Annotation #31

cbeaujoin opened this issue Sep 9, 2019 · 13 comments

Comments

@cbeaujoin
Copy link

cbeaujoin commented Sep 9, 2019

Hi,

for the moment we can easily handle Intent by their name:
@ForIntent("Intent Name")

As we often use webhook call for slot filling, it could be interesting to add a second parameter to the annotation that would handle the corresponding slot filling.
For example :
@ForIntent("Intent Name", "Parameter Name")

For the moment as a workaround I do like this :

 // Choose between slot filling
List<Context> outputContexts = request.getWebhookRequest().getQueryResult().getOutputContexts();
Optional<Context> param = outputContexts.stream().filter(context -> context.getName().contains("intent_name_dialog_params")).findFirst();
if (param.isPresent()) {
     if (param.get().getName().contains("parameter_name")) {
          // do this ..
      } else {
          ...
      }
}
@taycaldwell
Copy link
Contributor

Hi @cbeaujoin,

Thank you for your feature request! We are always looking on ways to improve our client libraries, so we appreciate feedback and suggestions for the community.

@spenc53
Copy link

spenc53 commented Oct 11, 2019

Should it only check for 1 parameter? Or should it also be able to take in a list of parameters?

@spenc53
Copy link

spenc53 commented Oct 11, 2019

Also, as a follow up for this, maybe at some point, add another annotation for @Param("paramName") paramName: string for functions to pass in data? Probably won't be part of this issue, might be out of scope.

@Fleker
Copy link
Member

Fleker commented Oct 14, 2019

If you do support a list of parameters, would your annotation support a comma-separated list of args or an array primitive?

@ForIntent("Intent Name", "Param Name 1", "Param Name 2", ...)

or

@ForIntent("Intent Name", ["Param Name 1", "Param Name 2"])

@spenc53
Copy link

spenc53 commented Oct 14, 2019

It could probably go either way on the array formatting. I think that an actual array would be more explicit. Also, to get which parameters to check,how should we pick which name from the outputContext to choose? Or do we look at the parameters for all of them? I provided the snippet below to show the output contexts and their names.

      {
        "name": "projects/projectId/agent/sessions/sessionId/contexts/choose_fact-followup",
        "lifespanCount": 5,
        "parameters": {
          "category.original": "",
          "category": "history"
        }
      },
      {
        "name": "projects/projectId/agent/sessions/sessionId/contexts/_actions_on_google",
        "lifespanCount": 98,
        "parameters": {
          "data": "{\"headquarters\":[\"google_headquarters_fact_1\",\"google_headquarters_fact_2\",\"google_headquarters_fact_3\"],\"cats\":[\"cat_fact_1\",\"cat_fact_2\",\"cat_fact_3\"],\"history\":[],\"test\":\"hello\"}",
          "category.original": "",
          "category": "history"
        }
      }
    ]```

@Fleker
Copy link
Member

Fleker commented Oct 14, 2019

You mean between "category" and "category.original"?

@spenc53
Copy link

spenc53 commented Oct 14, 2019

Oh, sorry, I should've explained better.

The @ForIntent("Intent Name") matches the top level intent name.
For the specific outputContexts, they also have names that are specific, will the parameters be the same for those?

@Fleker
Copy link
Member

Fleker commented Oct 14, 2019

You mean the parameter names, or the names of the context?

@spenc53
Copy link

spenc53 commented Oct 14, 2019 via email

@Fleker
Copy link
Member

Fleker commented Oct 14, 2019

I guess choosing either the first (exit early) or the last (replace) would be the primary expected way to do it. Or maybe throw an exception if not specific enough.

There are many different ways to do it which could all be potentially valid.

@spenc53
Copy link

spenc53 commented Oct 14, 2019

Would the user also need to specify the context type name then (just the last part)? And if they didn't care which it hit, leave it empty and it will default to match the first one that has all the parameters, if any?

@Fleker
Copy link
Member

Fleker commented Oct 14, 2019

Well I don't know whether they'd need to specify the context always. Certainly it's something you may need. But what do you think the expected behavior should be if it's empty?

@cbeaujoin
Copy link
Author

cbeaujoin commented Oct 15, 2019

Hi,

I don't think we need to support a list of parameters.
A second parameter (a String) is enough.

First let me add my understanding of the contexts names :

With actions we have a context at conversation level :

  • "projects//agent/sessions//contexts/_actions_on_google" built by the actions sdk and that should persist until end of conversation, it has an extended lifespan (99)

For a given intent "intent" whatever the number of filled params there is parent context :

  • "projects//agent/sessions//contexts/intent_dialog_context" that should persist until the user answer all the required params or leave the intent, its lifespan (1) is renewed by dialogflow

For a given intent "intent" with a required parameter "cellphone" not filled and in first position in the required parameters sorted list. There is a context :

  • "projects//agent/sessions//contexts/intent_dialog_params_cellphone"that should persist until the user answer the param or leave the intent, its lifespan (1) is renewed by dialogflow.
    It means we have to prompt the user for its cellphone number.

If there is no context that contains "dialog_params" it means we have reach the intent's end.

If we apply this to the annotation :

@ForIntent("Intent Name", "cellphone")
If a context that contains "intent_dialog_params_cellphone" exists :

  • it means we have to prompt the user for the cellphone number.

@ForIntent("Intent Name") => with an empty/null second parameters
If there is no context that contains "dialog_params":

  • It matchs an intent without parameters
  • It matchs an intent where all required parameters are filled, it means we have reach the intent's end.

or there is a dialog_params but it does not match any @ForIntent("Intent Name", "param")

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

Successfully merging a pull request may close this issue.

4 participants