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

Default naming scheme, behavior for all concepts #778

Closed
vishwacsena opened this issue Aug 31, 2019 · 11 comments
Closed

Default naming scheme, behavior for all concepts #778

vishwacsena opened this issue Aug 31, 2019 · 11 comments
Assignees
Labels
Area: LU Belongs to LU feature area P0 Must Fix. Release-blocker R10 Release 10 - August 17th, 2020 Type: Engineering

Comments

@vishwacsena
Copy link
Contributor

vishwacsena commented Aug 31, 2019

Default names

This specification outlines the default naming convention for all concepts in composer.

All triggers, actions start with a default name but is user editable.

Bot

There is no default name for a Bot. User must explictly provide a name.

Dialog

There is no default name for a dialog. User must explicitly provide a name.

Trigger

  • Only Intent trigger has a name property.
  • Default name for a trigger will be its type name - e.g. IntentTrigger
  • In case there already exists a trigger with the name, add a number increment as suffix - e.g. IntentTrigger, IntentTrigger1, IntentTrigger2, ...
  • All trigger names within a specific dialog must be unique.
  • In case of intent trigger, the intent name will default to the trigger name.
  • If user renames the trigger in the trigger's property editor, then the intent name is also updated.
  • If user changes the intent name of a trigger in all-up view and/ or if this intent is used as a reference in another intent, then ask the user for the intended behavior -
    • Update all references to the new name
    • Keep reference as is

Input/ prompt actions

  • Default name for an input action will be its type name - e.g. NumberInput, TextInput, ChoiceInput, ConfirmInput, DateTimeInput, OAuthInput, AttachmentInput
  • In case there already exists an input action with that name, add a number increment as suffix - e.g. NumberInput, NumberInput1, NumberInput2, ...
  • All input names tied to a specific dialog must be unique.
  • The intent name for an input default to the input name.
  • The prompt LG template name defaults to the input name.
  • The unrecognized prompt LG template name defaults to <input-name>.unrecognized.prompt
  • The invalid prompt LG template name defaults to <input-name>.invalid.prompt
  • If user renames the input in the input's property editor, then the intent name is also updated, LG template names are also updated.
  • If user changes the intent name of an input in the all-up view or changes the LG template name tied to an input in the all-up view and/ or if the intent or lG template is used as a reference in another intent/ LG template, then ask the user for the intended behavior -
    • Update all references to the new name
    • Keep reference as is

SendActivity

  • Default name is sendActivity, add an increment as suffix to de-dupe - e.g. SendActivity, SendActivity1, SendActivity2, ...
  • The default LG template name is the name of the SendActivity action
  • If user renames the send activity in its property editor, then the LG template name is also updated.
  • If user changes the LG template name in all-up view and/ or if this LG template is used as a reference in other intents or templates, then ask the user for the intended behavior -
    • Update all references to the new name
    • Keep reference as is

All other actions

  • Name is available, defaults to ActionType, add a number increment suffix to de-dupe
  • These names are just for organizational purposes and readaibility and do not have any runtime implications.

Note: This impacts the current naming scheme used for LG templates in send activity action. With this change, we will no longer use our current naming scheme bfd-<ID>

@yochay
Copy link
Contributor

yochay commented Sep 3, 2019

Default name for a trigger will be its type name - e.g. IntentTrigger, UnknownIntentTrigger, EventTrigger, ConversationUpdateTrigger.

  • Can we have multiple ConversationUpdateTrigger or UnknownIntentTrigger triggers? If not, we should 1) call that out and make sure we don't show / allow conflict configuration; 2) call out which triggers can be multiple, like Event and Intent.

The intent name for an input default to the input name.
The prompt LG template name defaults to the input name.

  • Do we add sofix or prefix for the LG for a given intent? In the future, when searching for a 'term' search results might find multiple object across different location. Do we care?

for SendActivity

  • today, all LG is in one Common file, we'll have template name collision with SendActivity1 ...SendActivityN. Should we add a dialog prefix/ sofix or something else?
  • it will be good to claim all SendActivity templates have unique name across the entire app. This will also help in the future with search and other language features like multi-lang

FYI - @vishwacsena , @benbrown , @cwhitten , @sangwoohaan

@vishwacsena
Copy link
Contributor Author

vishwacsena commented Sep 3, 2019

@yochay

Can we have multiple ConversationUpdateTrigger or UnknownIntentTrigger triggers? If not, we should 1) call that out and make sure we don't show / allow conflict configuration; 2) call out which triggers can be multiple, like Event and Intent.

Yes. You can albeit with slightly different constraints. In fact all our triggers support constraints and so you can even have the same intent event with slightly different constraints.

Do we add sofix or prefix for the LG for a given intent? In the future, when searching for a 'term' search results might find multiple object across different location. Do we care?

I'd think search we support (in the future) is able to present content grouped by area - e.g. you could in theory have a dialog name and intent name be the same and so even outstide of LG we need the ability for search results to indicate what type of content was a match.

today, all LG is in one Common file, we'll have template name collision with SendActivity1 ...SendActivityN. Should we add a dialog prefix/ sofix or something else?
it will be good to claim all SendActivity templates have unique name across the entire app. This will also help in the future with search and other language features like multi-lang

Good call about our current LG situation. We need to move to one .lg file per dialog for public preview. I'll file a separate ticket to track that. Also in general, these are just the default names that we go with and are user editable/ configurable. If I need better search results, I'd name them in a way I can identify them and so the user is in control. Like I said above for search though, we should plan and design for search results to include type (dialog, LU, LG, settings etc) of the matched content. Should there be more than one match in the same type, then we should indicate the containing dialog name. Filed this ticket to track the move to one .lg file per dialog.

@cwhitten
Copy link
Member

Vishwac to get updated scope

@vishwacsena
Copy link
Contributor Author

@cwhitten - discussed and updated scope w/ @sangwoohaan.

@sangwoohaan - can you please add link to the design?

@cwhitten - leaving this assigned to you for resourcing.

@vishwacsena vishwacsena assigned cwhitten and unassigned vishwacsena Oct 1, 2019
@sangwoohaan
Copy link
Contributor

Related issue raised during Hackathon #976

@sangwoohaan
Copy link
Contributor

LU/LG name changing UX flow (in the magenta boxes) :https://www.figma.com/file/04DymSq4xElhOBdAyutvvTbq/Composer-SH-06.19?node-id=1741%3A910

@vishwacsena vishwacsena added the Area: LU Belongs to LU feature area label Oct 2, 2019
@a-b-r-o-w-n
Copy link
Contributor

What if we also prepended the trigger name to templates defined within that trigger?

@sangwoohaan
Copy link
Contributor

I like @a-b-r-o-w-n 's suggestion. Just note that we also need to append additional texts to some templates like invalid or unrecognized prompts.

@cwhitten cwhitten removed the 4.6 label Oct 11, 2019
@hibrenda hibrenda added the R8 Release 8 - March 16th, 2020 label Dec 30, 2019
@cwhitten cwhitten added R9 Release 9 - May 15th, 2020 and removed R8 Release 8 - March 16th, 2020 R9-punt labels Mar 4, 2020
@cwhitten cwhitten added the P0 Must Fix. Release-blocker label Mar 21, 2020
@boydc2014
Copy link
Contributor

@cwhitten

As i go over this again, i don't see this as a P0, even if, probably we should delivery lg\lu copy\paste\delete first and then get back to this. Here is the reason

  1. No feature is blocked on this naming convention. Current id link between dialog node to lg\lu sections gave us a robust way to correlate already.

One of the design purpose when this spec is delivered to help correlate dialog nodes and lg\lu sections, which is not a problem now.

  1. Readability.
    This do helps readability, for sure. But while adding readability, this spec will introduce a dependency of dialog context when doing id generation. For example, the lu intent name will be numbered and prefixed with the trigger name.

This kind of dependency and tight relationship are
a) unstable, and require lots of efforts to maintain, think about move action up and down, or copy\paste new actions in, or deletion, it requires lots of effort to make it right, or we are aligned to spec and become confusing
b) fragile. Even if we either be smart enough to make it right in a), if any bug cause a little mistach, then the entire bot could be in an inconsistent state, and hard to recover automatically.

So i would strongly prefer we keep a unique id that is not contextual and can correlate easily. If we want more readability , we can add prefex, which is actually no much difference with we did today, TextInput-123456.

  1. If we really like the simplicity of HelloTrigger.TextInput1, I would suggest we do this after we support all lg\lu related change when copy\paste\delete first. Then this becomes a pure refactor.

If we bring this in immediately, it's refactor and ship features toghether, which is always hard and error-prone.

@cwhitten cwhitten added R10 Release 10 - August 17th, 2020 and removed R9 Release 9 - May 15th, 2020 labels Apr 12, 2020
@cwhitten
Copy link
Member

cwhitten commented Apr 12, 2020

hey @boydc2014 I've asked @zhixzhan to handle the R9 changes we want to make associated with this here: #2597

Moving this to R10. What is missing is the renaming behavior, which may or may not make it in.

@boydc2014
Copy link
Contributor

hey @boydc2014 I've asked @zhixzhan to handle the R9 changes we want to make associated with this here: #2597

Moving this to R10. What is missing is the renaming behavior, which may or may not make it in.

I think what this spec described is largely implemented, except two minor things

  1. we didn't really named input using number like 1,2,3, we use an random id as suffix.
  2. we didn't auto rename intents when user renaming trigger, which we triaged as too tricky to do.

so i think this one can claimed as complete in r10, let's create new issue in r11, if we still think something can be done in this area

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: LU Belongs to LU feature area P0 Must Fix. Release-blocker R10 Release 10 - August 17th, 2020 Type: Engineering
Projects
None yet
Development

No branches or pull requests

9 participants