Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Commit d2c102f

Browse files
feich-msfeich-ms
authored
Allow intent or entities using phraseList features of same name (#1089)
* support output to file for kb:export command * support name duplication of intent or entity with phraseList * fix comments and typo
1 parent 2b00408 commit d2c102f

File tree

3 files changed

+215
-23
lines changed

3 files changed

+215
-23
lines changed

‎packages/lu/src/parser/lufile/parseFileContents.js‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -479,9 +479,10 @@ const validateFeatureAssignment = function(srcItemType, srcItemName, tgtFeatureT
479479
* @param {String} featureType
480480
* @param {Object} range
481481
*/
482-
const addFeatures = function(tgtItem, feature, featureType, range, featureProperties) {
482+
const addFeatures = function(tgtItem, feature, featureType, range, featureProperties, featureIsPhraseList = false) {
483483
// target item cannot have the same name as the feature name
484-
if (tgtItem.name === feature) {
484+
// the only exception case is intent and entity can have same name with phraseList feature
485+
if (tgtItem.name === feature && !featureIsPhraseList) {
485486
// Item must be defined before being added as a feature.
486487
let errorMsg = `Source and target cannot be the same for usesFeature. e.g. x usesFeature x is invalid. "${tgtItem.name}" usesFeature "${feature}" is invalid.`;
487488
let error = BuildDiagnostic({
@@ -490,19 +491,22 @@ const addFeatures = function(tgtItem, feature, featureType, range, featureProper
490491
})
491492
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
492493
}
493-
let featureAlreadyDefined = (tgtItem.features || []).find(item => item.modelName == feature || item.featureName == feature);
494+
let featureToModelAlreadyDefined = (tgtItem.features || []).find(item => item.featureName == feature);
495+
let modelToFeatureAlreadyDefined = (tgtItem.features || []).find(item => item.modelName == feature);
494496
switch (featureType) {
495497
case featureTypeEnum.featureToModel: {
496498
if (tgtItem.features) {
497-
if (!featureAlreadyDefined) tgtItem.features.push(new helperClass.featureToModel(feature, featureProperties));
499+
if (!featureToModelAlreadyDefined) tgtItem.features.push(new helperClass.featureToModel(feature, featureProperties));
498500
} else {
499501
tgtItem.features = new Array(new helperClass.featureToModel(feature, featureProperties));
500502
}
501503
break;
502504
}
503505
case featureTypeEnum.modelToFeature: {
504506
if (tgtItem.features) {
505-
if (!featureAlreadyDefined) tgtItem.features.push(new helperClass.modelToFeature(feature, featureProperties));
507+
if (!modelToFeatureAlreadyDefined){
508+
tgtItem.features.push(new helperClass.modelToFeature(feature, featureProperties));
509+
}
506510
} else {
507511
tgtItem.features = new Array(new helperClass.modelToFeature(feature, featureProperties));
508512
}
@@ -543,14 +547,31 @@ const parseFeatureSections = function(parsedContent, featuresToProcess, config)
543547
if (intentExists !== undefined) {
544548
// verify the list of features requested have all been defined.
545549
let featuresList = section.Features.split(/[,;]/g).map(item => item.trim().replace(/^[\'\"]|[\'\"]$/g, ""));
550+
let featuresVisited = new Set();
546551
(featuresList || []).forEach(feature => {
547-
let entityExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => item.name == feature || item.name == `${feature}(interchangeable)`);
552+
// usually phraseList has higher priority when searching the existing entities as phraseList is more likely to be used as feature
553+
// but when a ml entity and a phraseList have same name, this assumption will cause a problem in situation that
554+
// users actually want to specify the ml entity as feature rather than phraseList
555+
// currently we can not distinguish them in lu file, as for @ intent A usesFeature B, B can be a ml entity or a phraseList with same name
556+
// the lu format for usesFeatures defintion need to be updated if we want to resolve this confusion completely
557+
let entityExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => (item.name == feature || item.name == `${feature}(interchangeable)`) && item.type == EntityTypeEnum.PHRASELIST);
558+
// if phraseList is not matched to the feature, search other non phraseList entities
559+
// or this intent use multiple features with same name, e.g., @ intent A usesFeatures B, B.
560+
// and current loop is processiong the second feature B
561+
// this is allowed in luis portal, you can add ml entity and phraseList features of same name to an intent
562+
// the exported intent useFeatures will have two features with same name listed, just like above sample @ intent A usesFeatures B, B
563+
if (!entityExists || featuresVisited.has(feature)) {
564+
entityExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => item.name == feature && item.type !== EntityTypeEnum.PHRASELIST);
565+
}
566+
567+
if (!featuresVisited.has(feature)) featuresVisited.add(feature);
568+
548569
let featureIntentExists = (parsedContent.LUISJsonStructure.intents || []).find(item => item.name == feature);
549570
if (entityExists) {
550571
if (entityExists.type === EntityTypeEnum.PHRASELIST) {
551572
// de-dupe and add features to intent.
552573
validateFeatureAssignment(section.Type, section.Name, entityExists.type, feature, section.Range);
553-
addFeatures(intentExists, feature, featureTypeEnum.featureToModel, section.Range, featureProperties.phraseListFeature);
574+
addFeatures(intentExists, feature, featureTypeEnum.featureToModel, section.Range, featureProperties.phraseListFeature, true);
554575
// set enabledForAllModels on this phrase list
555576
let plEnity = parsedContent.LUISJsonStructure.model_features.find(item => item.name == feature);
556577
if (plEnity.enabledForAllModels === undefined) plEnity.enabledForAllModels = false;
@@ -588,17 +609,34 @@ const parseFeatureSections = function(parsedContent, featuresToProcess, config)
588609
// Find the source entity from the collection and get its type
589610
let srcEntityInFlatList = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => item.name == section.Name);
590611
let entityType = srcEntityInFlatList ? srcEntityInFlatList.type : undefined;
612+
let featuresVisited = new Set();
591613
(featuresList || []).forEach(feature => {
592614
feature = feature.replace(/[\'\"]/g, "");
593-
let featureExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => item.name == feature || item.name == `${feature}(interchangeable)`);
615+
// usually phraseList has higher priority when searching the existing entities as phraseList is more likely to be used as feature to a entity
616+
// but when a ml entity and a phraseList have same name, this assumption will cause a problem in situation that
617+
// users actually want to specify the ml entity as feature rather than phraseList
618+
// currently we can not distinguish them in lu file, as for @ ml A usesFeature B, B can be another ml entity or a phraseList with same name
619+
// the lu format for usesFeatures defintion need to be updated if we want to resolve this confusion completely
620+
let featureExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => (item.name == feature || item.name == `${feature}(interchangeable)`) && item.type == EntityTypeEnum.PHRASELIST);
621+
// if phraseList is not matched to the feature, search other non phraseList entities
622+
// or this intent use multiple features with same name, e.g., @ intent A usesFeatures B, B.
623+
// and current loop is processiong the second feature B
624+
// this is allowed in luis portal, you can add ml entity and phraseList features of same name to an intent
625+
// the exported intent useFeatures will have two features with same name listed, just like above sample @ intent A usesFeatures B, B
626+
if (!featureExists || featuresVisited.has(feature)) {
627+
featureExists = (parsedContent.LUISJsonStructure.flatListOfEntityAndRoles || []).find(item => item.name == feature && item.type !== EntityTypeEnum.PHRASELIST);
628+
}
629+
630+
if (!featuresVisited.has(feature)) featuresVisited.add(feature);
631+
594632
let featureIntentExists = (parsedContent.LUISJsonStructure.intents || []).find(item => item.name == feature);
595633
// find the entity based on its type.
596634
let srcEntity = (parsedContent.LUISJsonStructure[luisEntityTypeMap[entityType]] || []).find(item => item.name == section.Name);
597635
if (featureExists) {
598636
if (featureExists.type === EntityTypeEnum.PHRASELIST) {
599637
// de-dupe and add features to intent.
600638
validateFeatureAssignment(entityType, section.Name, featureExists.type, feature, section.Range);
601-
addFeatures(srcEntity, feature, featureTypeEnum.featureToModel, section.Range, featureProperties.phraseListFeature);
639+
addFeatures(srcEntity, feature, featureTypeEnum.featureToModel, section.Range, featureProperties.phraseListFeature, true);
602640
// set enabledForAllModels on this phrase list
603641
let plEnity = parsedContent.LUISJsonStructure.model_features.find(item => item.name == feature);
604642
if (plEnity.enabledForAllModels === undefined) plEnity.enabledForAllModels = false;
@@ -635,14 +673,15 @@ const updateDependencyList = function(type, parsedContent, dependencyList) {
635673
let srcName = itemOfType.name;
636674
let copySrc, copyValue;
637675
if (itemOfType.features) {
638-
(itemOfType.features || []).forEach(feature => {
639-
if (feature.modelName) feature = feature.modelName;
640-
if (feature.featureName) feature = feature.featureName;
676+
(itemOfType.features || []).forEach(featureObj => {
677+
let feature = featureObj.modelName ? featureObj.modelName : featureObj.featureName;
678+
let type = featureObj.modelType ? featureObj.modelType : featureObj.featureType;
679+
641680
// find any items where this feature is the target
642-
let featureDependencyEx = dependencyList.filter(item => srcName == (item.value ? item.value.slice(-1)[0] : undefined));
681+
let featureDependencyEx = dependencyList.filter(item => srcName == (item.value ? item.value.slice(-1)[0].feature : undefined));
643682
(featureDependencyEx || []).forEach(item => {
644683
item.key = `${item.key.split('::')[0]}::${feature}`;
645-
item.value.push(feature);
684+
item.value.push({feature, type});
646685
})
647686
// find any items where this feature is the source
648687
featureDependencyEx = dependencyList.find(item => feature == (item.value ? item.value.slice(0)[0] : undefined));
@@ -653,19 +692,23 @@ const updateDependencyList = function(type, parsedContent, dependencyList) {
653692
let dependencyExists = dependencyList.find(item => item.key == `${srcName}::${feature}`);
654693
if (!dependencyExists) {
655694
let lKey = copySrc ? `${srcName}::${copySrc}` : `${srcName}::${feature}`;
656-
let lValue = [srcName, feature];
695+
let lValue = [srcName, {feature, type}];
657696
if (copyValue) copyValue.forEach(item => lValue.push(item));
658697
dependencyList.push({
659698
key : lKey,
660699
value : lValue
661700
})
662701
} else {
663702
dependencyExists.key = `${dependencyExists.key.split('::')[0]}::${feature}`;
664-
dependencyExists.value.push(feature);
703+
dependencyExists.value.push({feature, type});
665704
}
666-
let circularItemFound = dependencyList.find(item => item.value && item.value.slice(0)[0] == item.value.slice(-1)[0]);
667-
if (circularItemFound) {
668-
const errorMsg = `Circular dependency found for usesFeature. ${circularItemFound.value.join(' -> ')}`;
705+
706+
let circularItemFound = dependencyList.find(item => item.value
707+
&& item.value.slice(0)[0] == item.value.slice(-1)[0].feature
708+
&& item.value.slice(-1)[0].type !== featureProperties.phraseListFeature);
709+
710+
if (circularItemFound) {
711+
const errorMsg = `Circular dependency found for usesFeature. ${circularItemFound.value.map(v => v.feature ? v.feature : v).join(' -> ')}`;
669712
const error = BuildDiagnostic({
670713
message: errorMsg
671714
});

‎packages/lu/src/parser/utils/helpers.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,11 +561,14 @@ const addIsRequiredProperty = function(item, phraseListInFinal = []) {
561561
(item.features || []).forEach(feature => {
562562
if (feature.isRequired === undefined)
563563
feature.isRequired = false;
564-
if (feature.modelName !== undefined && phraseListInFinal.includes(feature.modelName))
565-
{
564+
565+
if (feature.modelName !== undefined
566+
&& phraseListInFinal.includes(feature.modelName)
567+
&& !item.features.find(fea => fea.featureName == feature.modelName)) {
566568
feature.featureName = feature.modelName;
567569
delete feature.modelName;
568-
}
570+
}
571+
569572
delete feature.featureType;
570573
delete feature.modelType;
571574
});

0 commit comments

Comments
 (0)